issue_comments: 391596424

This data as json

html_url issue_url id node_id user created_at updated_at author_association body reactions performed_via_github_app issue
https://github.com/pydata/xarray/issues/2176#issuecomment-391596424 https://api.github.com/repos/pydata/xarray/issues/2176 391596424 MDEyOklzc3VlQ29tbWVudDM5MTU5NjQyNA== 11411331 2018-05-24T05:50:26Z 2018-05-24T05:50:26Z CONTRIBUTOR

Well, I'm certainly not trying to argue that the PhysArray implementation is the solution. It's just a solution, and a solution for a much smaller problem.

I understand your concerns about cf_units. I've heard many people make the same argument for pint, and I appreciate it. I went with cf_units because of its dependence on UDUNITS, which is declared as the "standard" in the CF Conventions. I never had the time (nor do I foresee ever having the time) to see if pint was fully compliant with UDUNITS, so I went with something I knew was.

I personally think it's fine to discuss this here, unless other people would like to see this go offline. To address some of your issues:

1) As a container rather than subclass, it does not implement many of the methods of DataArray

Yes. I agree. In fact, my first implementation was a subclass of DataArray, but after reading the Xarray documentation here and the Issue #706, I decided on composition for this implementation.

2) There are a few design choices I don't understand, like why calendar is always a property of a PhysArray even when it isn't storing a time

I personally didn't see much of a benefit to a check like hasattr(obj, 'calendar') versus obj.calendar is None (other than the fact that these two checks are opposite). So, for my code, I felt that obj.calendar is None was a reasonable check to see if the units were "calendared time" units.

why cftime objects aren't used instead of units to manage time

As I mentioned in my post, this is just my personal preference. I think that calendared time units are...well...just units, and that the same mechanism for dealing with all other units should deal with calendared time units. It's just an aesthetic that I prefer. (That said, I'm extremely happy that someone finally dealt with non-standard calendars with cftime.)

why the positive attribute is important enough for PhysArray to manage (I've never seen it in any data I've used, and it's easy to check if a DataArray is all positive or negative with a function call)

The CF Conventions define the positive attribute to indicate that the field represents the vertical component of a vector, and the value of positive indicates what direction the vertical component points (up or down) if the data is positive. So, if you add field X and field Y, and X.positive == 'up' but Y.positive == 'down', then you need to actually subtract the fields. It's an annoying attribute that I've had to deal with when preparing data for CMIP6.

In the end, I'm happy doing whatever the community wants with this code. I can pull it out into it's own repo (along with the unit tests, which are also in the PyConform repo). And then, after that, I have no problem with people taking it in a different direction (e.g., using pint, injecting properties based on the value of the attributes, etc.). I'm also happy with a subclass option, as my C++ experiences in the past have made me very comfortable with inheritance. I take guidance easily. I just don't usually have a lot of time to devote to development any more these days. :-(

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   325810810