Attribute: Implement get<>() Cast by ax3l · Pull Request #345 · openPMD/openPMD-api

ax3l · 2018-09-13T20:40:36Z

Close #335: Implement getCast<>() in Attribute::get<U>(). Will now cast
during ::get<U>() if convertible.

Implementations can now simply use ::get<>() again :)
Final clean-up for refactoring started in #337

ax3l · 2018-09-14T06:22:15Z

@C0nsultant Ping :)

anokfireball · 2018-09-14T07:30:20Z

This make Attributable::readFloatingPoint() redundant. The casting logic applied therein is now handled in get itself. I'll do a follow-up PR that removes this redundancy and just uses get.

anokfireball · 2018-09-14T07:32:57Z

include/openPMD/backend/Attribute.hpp

+template< typename U >
+U Attribute::get() const
+{
+    return getCast< U >( Variant::getResource() );


I would hate to cast silently here and not notify the user about possible loss of precision.
There already is a function that does this (warnWrongDtype()). I'll push a patch shortly that will use that function if the resource dtype and U differ.

That's actually not what I would like as a user.
If there is an attribute "attr" I can RT check it's dtype. If I want to get<> it as something, I want to get it as exactly such.

Let's have a quick chat on the philosophy of that method if you like :) Gitter? VC?
In case you are busy today, we can also chat on Monday. I have to leave here around noon.

Offline resolution:
C++ requires a certain effort from the user to put the type inside the brackets. Thus, he should be well aware of what he is requesting.
Information about the datatype can be obtained at run-time. If non-casting behavior is desired, we shift this obligation to the user.

If non-casting behavior is desired, we shift this obligation to the user.

e.g. by checking the dtype at runtime, taking the resource directly or casting back to the underlying auxiliary::Variant that will throw.

anokfireball · 2018-09-14T07:33:37Z

test/SerialIOTest.cpp

-    REQUIRE(getCast< std::vector< long double > >(s.getAttribute("vecLongdouble")) == std::vector< long double >({0.L, std::numeric_limits<long double>::max()}));
+    REQUIRE(s.getAttribute("vecLongdouble").get< std::vector< long double > >() == std::vector< long double >({0.L, std::numeric_limits<long double>::max()}));
    REQUIRE(s.getAttribute("vecString").get< std::vector< std::string > >() == std::vector< std::string >({"vector", "of", "strings"}));
    // REQUIRE(s.getAttribute("bool").get< bool >() == true);


This should work as expected now, as the stored uint8_t can be cast to bool in the new get.

Just need to rebase, done already in #344

Implement `getCast<>()` in `Attribute::get<U>()`. Will now cast during get if convertible.

getCast is now convieniently implemented in ::get

Added this for being pedantic in case this causes a warning. Nevertheless, would have to create a factory here to properly init a `std::array`.

anokfireball · 2018-09-14T09:04:39Z

test/SerialIOTest.cpp

+    REQUIRE(s.getAttribute("vecLongdouble").get< std::vector< long double > >() == std::vector< long double >({0.L, std::numeric_limits<long double>::max()}));
    REQUIRE(s.getAttribute("vecString").get< std::vector< std::string > >() == std::vector< std::string >({"vector", "of", "strings"}));
    REQUIRE(s.getAttribute("bool").get< unsigned char >() == true);
    REQUIRE(s.getAttribute("boolF").get< unsigned char >() == false);


This now can be retrieved as bool, thanks to the casting get.

Have this in my staged set, will push soon.

That sounds great, thanks! :)

ax3l · 2018-09-14T09:51:01Z

src/Datatype.cpp

+                    Datatype store,
+                    Datatype request)
+{
+    std::cerr << "Warning: Attribute '" << key


loving MSVC:

error C2679: binary '<<': no operator found which takes a right-hand operand of type 'const std::string' (or there is no acceptable conversion

forgot to include <string>

fixed it :)

The handling an casting of arbitrary-width floating-point Attributes is now redundant with the casting that is present in Attrbute::get.

In `pybind11`, overloads on types are order-dependent (first wins). pybind/pybind11#1512 We specialize `double` here generically and cast in read if needed (see openPMD#345 openPMD#1137). Later on, we could add support for 1D numpy arrays with distinct type.

In `pybind11`, overloads on types are order-dependent (first wins). pybind/pybind11#1512 We specialize `double` here generically and cast in read if needed (see #345 #1137). Later on, we could add support for 1D numpy arrays with distinct type.

ax3l added the refactoring label Sep 13, 2018

ax3l assigned anokfireball Sep 13, 2018

ax3l requested a review from anokfireball September 13, 2018 20:40

ax3l force-pushed the topic-getAttributeCast branch 3 times, most recently from 8ae9d20 to 45b7e04 Compare September 13, 2018 21:37

anokfireball suggested changes Sep 14, 2018

View reviewed changes

ax3l added 3 commits September 14, 2018 10:47

Attribute: Implement get<>() Cast

450c55e

Implement `getCast<>()` in `Attribute::get<U>()`. Will now cast during get if convertible.

Implementations: use Attribute::get<>

06f786e

getCast is now convieniently implemented in ::get

Attribute: Remove return after throw

78c7a2f

Added this for being pedantic in case this causes a warning. Nevertheless, would have to create a factory here to properly init a `std::array`.

ax3l force-pushed the topic-getAttributeCast branch from 45b7e04 to 78c7a2f Compare September 14, 2018 08:48

anokfireball reviewed Sep 14, 2018

View reviewed changes

anokfireball force-pushed the topic-getAttributeCast branch from 50120b2 to 4b3a053 Compare September 14, 2018 09:16

anokfireball approved these changes Sep 14, 2018

View reviewed changes

ax3l commented Sep 14, 2018

View reviewed changes

Use new casting get to read fp Attributes

f67e00e

The handling an casting of arbitrary-width floating-point Attributes is now redundant with the casting that is present in Attrbute::get.

ax3l force-pushed the topic-getAttributeCast branch from 4b3a053 to f67e00e Compare September 14, 2018 09:52

ax3l merged commit 4da295b into openPMD:dev Sep 14, 2018

ax3l deleted the topic-getAttributeCast branch September 14, 2018 11:36

ax3l mentioned this pull request Jun 8, 2022

Python: grid_spacing & time_offset double #1290

Merged

Conversation

ax3l commented Sep 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ax3l commented Sep 14, 2018

Uh oh!

anokfireball commented Sep 14, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ax3l Sep 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ax3l commented Sep 13, 2018 •

edited

Loading

ax3l Sep 14, 2018 •

edited

Loading