Pyudunits2 #1118
Pyudunits2 #1118
Conversation
|
@pelson Windows failures apart due to the micromamba bug, and the hacks I made to get the dates to work, this is great! Pyudunits2 can replace cf-units with just a few minor tweaks IMO. |
|
This is a bit worrying. The test run time increased a lot with Develop branch: This branch: |
ocefpaf
force-pushed
the
pyudunits2
branch
3 times, most recently
from
c4a1619 to
85723cb
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1118 +/- ##
============================================
- Coverage 81.91% 71.25% -10.66%
============================================
Files 25 25
Lines 5224 5263 +39
Branches 1163 1169 +6
============================================
- Hits 4279 3750 -529
- Misses 644 1166 +522
- Partials 301 347 +46 ☔ View full report in Codecov by Sentry. |
compliance_checker/cf/cf_1_6.py
Outdated
| @@ -1864,8 +1865,7 @@ def check_time_coordinate(self, ds): | |||
| ret_val.append(result) | |||
| # IMPLEMENTATION CONFORMANCE 4.4 RECOMMENDED 2/2 | |||
| # catch non-recommended months or years time interval | |||
| unit = Unit(variable.units) | |||
| if unit.is_long_time_interval(): | |||
There was a problem hiding this comment.
@benjwadams this is deprecated in cf-units and the change below should be implemented regardless of this PR. I'll open a new one just for this soon.
compliance_checker/cfutil.py
Outdated
| @@ -2066,9 +2066,6 @@ def _units(units: str): | |||
| # FIXME: cf_units.Unit(None) -> Unit('unknown') | |||
| if units is None: | |||
| units = "" | |||
| # FIXME: Syntax Error when HH:MM:SS is present in time reference. | |||
| if "T00:00:00" in units: | |||
| units = units.replace("T00:00:00", "") | |||
There was a problem hiding this comment.
@pelson worked like a charm! All tests passing.
compliance_checker/cfutil.py
Outdated
| except (SyntaxError, UnresolvableUnitException) as err: | ||
| raise ValueError from err | ||
| # FIXME: cf_units defined .is_time_reference for time reference units. | ||
| u.is_time_reference = False |
There was a problem hiding this comment.
@pelson cf-units has a .is_time_reference() method to differentiate time units from reference time units (the COARDS format "time since date + a calendar") . Would something like this make sense in pyudunits2? It is easy enough for us to implement it. I guess that the question is: Will pyudunits2 be a replacement for cf-units or will cf-units use it under the hood and these methods should remain in cf-units.
compliance_checker/cfutil.py
Outdated
| # FIXME: hasattr should return None in that case. | ||
| # pyudunits2/_expr_graph.py:27, in Node.__getattr__(self, name) | ||
| # 25 def __getattr__(self, name): | ||
| # 26 # Allow the dictionary to raise KeyError if the key doesn't exist. |
There was a problem hiding this comment.
@pelson I would expect pyudunits2 to return False instead of raising here. Does that make sense?
ocefpaf
force-pushed
the
pyudunits2
branch
6 times, most recently
from
a5f04cc to
54fbfc8
Compare
Possible alternative for #1094
TODO:
compliance_checker/cfutil.py(_CCUnitsand_units) whenpyudunits2is released.