yt-project / unyt

Handle, manipulate, and convert data with units in Python
https://unyt.readthedocs.io
BSD 3-Clause "New" or "Revised" License
364 stars 48 forks source link

BUG: (NEP 18) np.histogram raises TypeError for range using implicit units #465

Closed neutrinoceros closed 11 months ago

neutrinoceros commented 11 months ago

Description

This is a regression in unyt 3.0.0, discovered in testing yt's cookbook.

What I Did

import unyt as un
import numpy as np

np.histogram(
    un.unyt_array([0, 1, 2, 3], 'yr'),
    bins=2,
    range=[0, 5e8], # years
)
Traceback (most recent call last):
  File "/Users/robcleme/dev/yt-project/yt/g.py", line 4, in <module>
    np.histogram(
  File "/Users/robcleme/dev/yt-project/yt/_unyt/unyt/array.py", line 2034, in __array_function__
    return _HANDLED_FUNCTIONS[func](*args, **kwargs)
  File "/Users/robcleme/dev/yt-project/yt/_unyt/unyt/_array_functions.py", line 164, in histogram
    range = _sanitize_range(range, units=[a.units])
  File "/Users/robcleme/dev/yt-project/yt/_unyt/unyt/_array_functions.py", line 149, in _sanitize_range
    raise TypeError(
TypeError: Elements of range must both have a 'units' attribute. Got [0, 500000000.0]

The error message, which is 100% intentional, hints a the preferred way to call this function by making units explicit:

import unyt as un
import numpy as np

np.histogram(
    un.unyt_array([0, 1, 2, 3], 'yr'),
    bins=2,
    range=un.unyt_array([0, 5e8], 'yr'),
)

however, this doesn't work with unyt 2.9.5. The only way to write this code that's portable against unyt 2.9.5 and unyt 3.0.0 is to strip units, which is less than ideal. In hindsight, I think it should still be allowed to use implicit units for the range argument here. I'll try to come up with a non-invasive fix.