Sledgehammer pass at refactoring units #1552
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main-dev #1552 +/- ##
============================================
- Coverage 78.11% 77.47% -0.64%
============================================
Files 145 141 -4
Lines 21579 20852 -727
============================================
- Hits 16856 16155 -701
+ Misses 4723 4697 -26
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| from pyaerocom.helpers import ( | ||
| get_lowest_resolution, | ||
| isnumeric, | ||
| make_datetime_index, | ||
| to_pandas_timestamp, |
There was a problem hiding this comment.
same here, surprising indeed that the import works
| @@ -23,11 +23,10 @@ | |||
| VariableDefinitionError, | |||
| VariableNotFoundError, | |||
| ) | |||
| from pyaerocom.units.datetime import cftime_to_datetime64, datetime2str | |||
| from pyaerocom.helpers import ( | |||
pyaerocom/colocation/colocator.py
Outdated
| from pyaerocom.helpers import ( | ||
| get_lowest_resolution, | ||
| start_stop, | ||
| to_datestring_YYYYMMDD, | ||
| to_pandas_timestamp, |
| @@ -7,8 +7,8 @@ | |||
| from pyaerocom import const | |||
| from pyaerocom.aeroval.modelentry import ModelEntry | |||
| from pyaerocom.griddeddata import GriddedData | |||
| from pyaerocom.units.datetime import get_highest_resolution | |||
| from pyaerocom.helpers import ( | |||
There was a problem hiding this comment.
here is the import, sorry, I deleted a comment which was in the wrong file
| def __radd__(self, other) -> MolecularMass: | ||
| return self.__add__(other) | ||
|
|
||
| def __sub__(self, other) -> MolecularMass: | ||
| if isinstance(other, MolecularMass): |
There was a problem hiding this comment.
to improve coverage would it be possible to add some dummy testing here, something like
def test_molecular_mass___sub___and___mul__():
mass = MolecularMass("H2")
mass2 = MolecularMass("O")
assert mass2.__sub__(mass).mass == 13.983519999999999
assert mass.__mul__(mass2) == 32.252870472
?
There was a problem hiding this comment.
coverage aside, I have no other comments otherwise, so I'll approve
| return str(self.val) | ||
|
|
||
|
|
||
| def sort_ts_types(ts_types: list[str | TsType]) -> list[str]: |
There was a problem hiding this comment.
Out of scope for this PR but it would be nice if all occurrences of time series types in pyaerocom were instances of TsType
Change Summary
A very rough attempt at collecting units related code in a new
unitssubmodule. For now this is mainly focused on moving existing code, not improving the code (I'll leave that to a second PR to make the reviews manageable). That being said, I've adressed a couple of low hanging fruits:@total_orderingdecorator.sort_ts_types,get_lowest_resolution,get_highest_resolutionusing Python's built insorted().calendar.isleap()to avoid Y2.1k problems :P_concprcp_units_helpers.pysince it isn't used (?). Most functions have raised NotImplementedErrors for ~3 years.MolecularMassclass to represent molar mass.Overview of moved things
pyaerocom/units_helpers.py->pyaerocom/units/units_helpers.pypyaercom/helpers.pysplit betweenpyaercom/units/datetime/utils.pyandpyaerocom/units/helpers.pypyaerocom/time_config.py->pyaerocom/units/datetime/_time_config.pypyaerocom/molmasses.py->pyaerocom/units/molecularmass.pyRelated issue number
related to #1512
Checklist