NCAS-CMS / cfunits

A Python interface to UNIDATA’s UDUNITS-2 library with CF extensions:
http://ncas-cms.github.io/cfunits
MIT License
11 stars 8 forks source link

Units.conform behavior varies depending on arguments #9

Closed JimBiardCics closed 4 years ago

JimBiardCics commented 4 years ago

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 commented 4 years ago

Thanks, Jim. I'll investigate ...

davidhassell commented 4 years ago

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?

JimBiardCics commented 4 years ago

That sounds great!

davidhassell commented 4 years ago

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