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

Units.conform behavior varies depending on arguments #9

Closed
JimBiardCics opened this issue Jul 1, 2020 · 4 comments
Closed

Units.conform behavior varies depending on arguments #9

JimBiardCics opened this issue Jul 1, 2020 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@JimBiardCics
Copy link

Great package!

I'm using cfunits 3.2.7 installed via conda from conda-forge (build pyh9f0ad1d_0) with Python 3.8.2.

The code below will produce an error on the first case, pass on the middle two, and fail on the last. There is different behavior depending on whether or not the from_units and to_units are the same or not.

from cfunits import Units

import numpy


# This fails.
#
try:
    print(Units.conform(35, Units('degrees_C'), Units('degrees_C')))
except Exception as exc:
    print(exc)

# This passes.
#
try:
    print(Units.conform([35], Units('degrees_C'), Units('degrees_C')))
except Exception as exc:
    print(exc)

value = numpy.array([35])

# This passes.
#
try:
    print(Units.conform(35, Units('degrees_C'), Units('degrees_F')))
except Exception as exc:
    print(exc)

# This fails.
#
try:
    print(Units.conform([35], Units('degrees_C'), Units('degrees_F')))
except Exception as exc:
    print(exc)
@davidhassell
Copy link
Contributor

Thanks, Jim. I'll investigate ...

@davidhassell
Copy link
Contributor

Hi Jim,

The first case (Units.conform(35, Units('degrees_C'), Units('degrees_C'))) was just a bug, now fixed.

In the fourth case (Units.conform([35], Units('degrees_C'), Units('degrees_F'))) you've found a feature, in that the conform method was documented to only work on numpy.ndarray or python numeric objects (https://github.com/NCAS-CMS/cfunits/blob/master/cfunits/units.py#L1902).

However, the code was clearly somewhat relaxed about this, and it is absolutely not unreasonable for it to work with lists or tuples as well. What is a bit harder is to get it to return and list or tuple when a one was provided (because it has to be converted to numpy internally to interact with the udunits2 C library), especially if an input tuple is nested.

I propose changing the code so that a list or tuple input always works, but always returns a numpy array in this case:

from cfunits import Units

import numpy


# This no longer fails (nothing to do with lists - just a bug!)
#
try:
    print(repr(Units.conform(35, Units('degrees_C'), Units('degrees_C'))))
except Exception as exc:
    print(exc)

# This still passes, but now returns a numpy array rather than a list
#
try:
    print(repr(Units.conform([35], Units('degrees_C'), Units('degrees_C'))))
except Exception as exc:
    print(exc)

# This still passes.
#
try:
    print(repr(Units.conform(35, Units('degrees_C'), Units('degrees_F'))))
except Exception as exc:
    print(exc)

# This now passes, and returns a numpy array
#
try:
    print(repr(Units.conform([35], Units('degrees_C'), Units('degrees_F'))))
except Exception as exc:
    print(exc)

prints:

35
array([35])
94.99999999999989
array([95.])

How does that sound?

@davidhassell davidhassell self-assigned this Jul 2, 2020
@davidhassell davidhassell added the bug Something isn't working label Jul 2, 2020
davidhassell added a commit to davidhassell/cfunits that referenced this issue Jul 2, 2020
davidhassell added a commit to davidhassell/cfunits that referenced this issue Jul 2, 2020
@JimBiardCics
Copy link
Author

That sounds great!

@davidhassell
Copy link
Contributor

Thanks - the new version, 3.2.8, will be ready in PyPi later today.

davidhassell added a commit that referenced this issue Jul 2, 2020
Fox for Units.conform behavior varies depending on arguments (#9)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants