Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feedback on the conversion and dimensionality functionality #3

Open
pelson opened this issue Nov 3, 2024 · 4 comments
Open

Feedback on the conversion and dimensionality functionality #3

pelson opened this issue Nov 3, 2024 · 4 comments

Comments

@pelson
Copy link
Owner

pelson commented Nov 3, 2024

Following the discussion in SciTools/cf-units#446 (comment), it would be good to get some feedback on how the implementation here for dimensionality and conversion works out for you @ocefpaf.

The README.md has some info, but for your is_convertible and is_dimensionless cases you can use the following:

from pyudunits2 import UnitSystem

ut_system = UnitSystem.from_udunits2_xml()

unit1 = ut_system.unit('km/h')
unit2 = ut_system.unit('meters per second')

assert unit1.is_convertible_to(unit2)
assert not unit1.is_convertible_to(ut_system.unit('m'))

And:

assert ut_system.unit('radians mg/kilogram').is_dimensionless()

There is an example in the README if you want to do actual conversions, rather than just knowing if conversions are possible.

@ocefpaf
Copy link

ocefpaf commented Nov 5, 2024

Phil, this is awesome. See ioos/compliance-checker#1118

That PR passes locally, for some reason the xml file is not installed when doing a pip+git install. Both is_dimensionless and is_convertible_to are working. (I did uncover a few bugs in our tests that I'll be fixing ASAP.)

@ocefpaf
Copy link

ocefpaf commented Nov 5, 2024

BTW, the .dimensionality() you mention in SciTools/cf-units#446 (comment) is great. Sadly my test there is not that simple. I do need to check if the unit is one of those in that list and not necessarily something that has dimensionality of meters. I'll have to re-think that part of the code.

@pelson
Copy link
Owner Author

pelson commented Nov 6, 2024

for some reason the xml file is not installed when doing a pip+git install

In your PR you are doing https://github.com/pelson/pyudunits2/archive/f75f1afd27d4b66ef77fc9db9301ae914111d288.zip. If that is the pip+git you mention above, it is because that isn't pip+git 😜 - I think the way I'm using setuptools-scm means that a plain tarball won't get the XML file installed. The following seems to work though: pip install git+https://github.com/pelson/pyudunits2.git@f75f1afd27d4b66ef77fc9db9301ae914111d288. Suggest opening an issue for this if it isn't resolved by the above and I will dig into it (or just publish to pypi to make your life easier 😉).

@ocefpaf
Copy link

ocefpaf commented Nov 6, 2024

it is because that isn't pip+git 😜

Indeed it is not. And here I thought that going for a tarball would be easier, I would miss only the version info... Let me try a real pip from git now.

Suggest opening an issue for this if it isn't resolved by the above and I will dig into it (or just publish to pypi to make your life easier 😉).

You don't have to do that just for this test. However, if you manage to implement the date parsing, not sure of that is on your roadmap, then a first release would be great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants