BAMWelDX / weldx

The welding data exchange format
https://www.bam.de/weldx
BSD 3-Clause "New" or "Revised" License
19 stars 9 forks source link

update converters for pint 0.22 #880

Closed braingram closed 1 year ago

braingram commented 1 year ago

in pint 0.22 Quantity appears at pint.registry.Quantity and requires an update to the corresponding asdf Converter (the same for pint.registry.Unit).

Changes

I opted to add the type pint.Quantity to the types supported by PintQuantityConverter. Types and/or strings can be used here and both have pros/cons. Using the type requires that pint be imported when the Converter is created (which can slow down initialization while asdf loads extensions. However this doesn't appear to be an issue here as pint is already imported when weldx.tags.units.pint_quantity is imported. By using the type, pint is free to move the internal implementation of Quantity (which can change the class path) but as long as it appears importable via pint.Quanity the Converter should stay up-to-date.

In this case the string "pint.Quantity" also needs to be included as weldx.constants.Q_ appears as a "pint.Quantity" when asdf inspects instances of weldx.constants.Q_ (as inspected with asdf.util.get_class_name).

I added some comments with a shorter version of the above description.

Related Issues

Fixes #879

Checks

github-actions[bot] commented 1 year ago

Test Results

2 188 tests  ±0   2 187 :heavy_check_mark: +202   2m 43s :stopwatch: +27s        1 suites ±0          1 :zzz: ±    0         1 files   ±0          0 :x:  - 197 

Results for commit 7e998ede. ± Comparison against base commit 65e63065.

:recycle: This comment has been updated with latest results.

codecov[bot] commented 1 year ago

Codecov Report

Merging #880 (7e998ed) into master (ac9ba0f) will decrease coverage by 0.02%. The diff coverage is 92.30%.

@@            Coverage Diff             @@
##           master     #880      +/-   ##
==========================================
- Coverage   96.48%   96.47%   -0.02%     
==========================================
  Files          95       95              
  Lines        6293     6293              
==========================================
- Hits         6072     6071       -1     
- Misses        221      222       +1     
Impacted Files Coverage Δ
weldx/tags/units/pint_quantity.py 97.29% <ø> (ø)
weldx/util/xarray.py 96.15% <ø> (ø)
weldx/core/time_series.py 97.74% <66.66%> (ø)
weldx/constants.py 100.00% <100.00%> (ø)
weldx/core/generic_series.py 89.73% <100.00%> (ø)
weldx/time.py 97.88% <100.00%> (ø)
weldx/transformations/rotation.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

CagtayFabry commented 1 year ago

thank you for the thorough explanation and solution with listing pint.Quantity as string and class for the converter @braingram

it is unfortunate that pint seems to be slightly unstable with the last releases in terms of typing and class refactoring.

I am waiting for the conda release of 0.22 here to merge this and update our dependencies and CI accordingly

braingram commented 1 year ago

Thanks!