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

BUG: issubdtype is inconsistent on types and dtypes #9505

Merged
merged 7 commits into from
Aug 6, 2017

Conversation

eric-wieser
Copy link
Member

@eric-wieser eric-wieser commented Aug 1, 2017

Previously, issubdtype(x, y) and issubdtype(dtype(x), dtype(y)) could give different results

Fixes #9480 and #9506 and #2334 (duped by #5711 and #3218)

This deprecates all of the following strange behaviors, with the intent in the distant future to invert the results:

>>> np.issubdtype(np.float32, 'float64')
True
>>> np.issubdtype(np.float32, 'f8')
True
>>> np.issubdtype(np.int32, basestring)
True
>>> np.issubdtype(np.int32, 'int64')
True
>>> np.issubdtype(np.unicode_, 'void')
True
>>> np.issubdtype(np.int32, int)          # should have been spelt np.integer
True
>>> np.issubdtype(np.float32, float)      # should have been spelt np.floating
True
>>> np.issubdtype(np.complex64, complex)  # should have been spelt np.complexfloating
True
>>> np.issubdtype(np.float32, "float")
True
>>> np.issubdtype(np.float64, "f")
True

@eric-wieser
Copy link
Member Author

test_numerictypes seemed like the obvious place to put a test for numerictypes.py.

However, if you actually look at that file, it seems to not be related to numerictypes.py at all

@eric-wieser eric-wieser force-pushed the fix-issubdtype branch 2 times, most recently from 6b9830f to d602521 Compare August 1, 2017 13:25
@charris
Copy link
Member

charris commented Aug 1, 2017

Yeah, the numpy type hierarchy is a mess. Not conceptually, but as implemented. There are things like

In [3]: dtype(np.integer)
Out[3]: dtype('int64')

that, IMHO, should raise errors. Perhaps we should make things like np.integer abstract classes now that those are available. The main problem with cleaning things up will be the number of backward compatibility problems that will surely arise. The issue should probably be raised on the mailing list because of its potential impact.

@eric-wieser
Copy link
Member Author

I've thought about this patch some more, and decided the best approach is just to fix dtype inputs, and FutureWarning in all other cases

@eric-wieser
Copy link
Member Author

Perhaps we should make things like np.integer abstract classes

They already are not creatable - np.integer(1) fails. Problem is that dtypes don't care whether their type has a constructor.

@eric-wieser
Copy link
Member Author

@charris: See updated PR. Apart from the DEP commit, which I can split off if needed, I don't think this needs a mailing list consultation

# float, and similar less obvious things, such as np.generic from
# basestring
mro = arg2.mro()
arg2 = mro[1] if len(mro) > 1 else mro[0]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note the only way that len(mro) == 1 is if arg2 is object, which should never happen - but we had it before too, so...

warnings.warn(
"Conversion of the second argument of issubdtype from `{raw}` "
"to `{abstract} is deprecated. In future, it will be treated as "
"`{concrete} == np.dtype({raw}).type`.".format(
Copy link
Member Author

@eric-wieser eric-wieser Aug 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example message:

In [1]: np.issubdtype(complex, bytearray)
True

FutureWarning: Conversion of the second argument of issubdtype from bytearray to np.generic is deprecated. In future, it will be treated as np.object_ == np.dtype(bytearray).type.

@eric-wieser
Copy link
Member Author

A quick search reveals that matplotlib makes the issubdtype(..., int) mistake in some rarely-used code (combined with the false assumption that np.int != int), and scipy makes a few of these too.

This will also catch out people who wrote np.float but meant np.floating, which scipy is guilty of

@eric-wieser eric-wieser force-pushed the fix-issubdtype branch 2 times, most recently from c666800 to 1e06594 Compare August 1, 2017 19:33
@charris
Copy link
Member

charris commented Aug 5, 2017

LGTM. Could you add an entry in the release notes under Future Changes.

@eric-wieser
Copy link
Member Author

Happy to rebase this myself later.

@charris
Copy link
Member

charris commented Aug 5, 2017

The test queue looks stuffed. Yeah, go ahead and fix things up and add the release note.

@charris
Copy link
Member

charris commented Aug 5, 2017

Close and reopen to see if the tests restart.

@charris charris closed this Aug 5, 2017
@charris charris reopened this Aug 5, 2017
In many cases, this coercion is surprising, or would be if the user knew about it:

* [('a', int)] -> np.flexible
* str - > str (!) - not even a numpy type
* 'float32' -> np.floating (discards size)
* int -> np.signed_integer (not np.integer, as is usually meant)
@charris
Copy link
Member

charris commented Aug 6, 2017

Thanks Eric.

@eric-wieser eric-wieser added this to the 1.14.0 release milestone Sep 27, 2017
effigies added a commit to effigies/h5py that referenced this pull request Oct 25, 2017
Numpy is deprecating this usage (see numpy/numpy#9505).
benjello added a commit to openfisca/openfisca-survey-manager that referenced this pull request Jan 30, 2018
ariddell pushed a commit to stan-dev/pystan2 that referenced this pull request Jan 31, 2018
Fixes issue #419 that prompts a FutureWarning due to a numpy change (as can be seen in numpy/numpy#9505)
bwohlberg added a commit to bwohlberg/sporco that referenced this pull request Feb 24, 2018
jhlegarreta added a commit to jhlegarreta/nibabel that referenced this pull request Dec 25, 2018
Fix future warning on use of `np.issubdtype`. Fixes:
```
/home/travis/build/nipy/dipy/venv/lib/python3.4/site-packages/nibabel/streamlines/array_sequence.py:23:
FutureWarning: Conversion of the second argument of issubdtype from `bool`
to `np.generic` is deprecated. In future, it will be treated as `np.bool_
== np.dtype(bool).type`.
```

observed in projects using Nibabel, e.g. DIPY:
https://travis-ci.org/nipy/dipy/jobs/471947136

More information about the issue and the fix at:
numpy/numpy#2334
numpy/numpy#9505
jhlegarreta added a commit to jhlegarreta/nibabel that referenced this pull request Dec 25, 2018
Fix future warning on use of `np.issubdtype`. Fixes:
```
/home/travis/build/nipy/dipy/venv/lib/python3.4/site-packages/nibabel/streamlines/array_sequence.py:23:
FutureWarning: Conversion of the second argument of issubdtype from `bool`
to `np.generic` is deprecated. In future, it will be treated as `np.bool_
== np.dtype(bool).type`.
```

observed in projects using Nibabel, e.g. DIPY:
https://travis-ci.org/nipy/dipy/jobs/471947136

More information about the issue and the fix at:
numpy/numpy#2334
numpy/numpy#9505
seberg added a commit to seberg/numpy that referenced this pull request Mar 18, 2020
This finishes the deprecation started in numpygh-9505 removing
behaviour that allowed strings/types representing specific dtypes
to behave like their more generic supertypes (e.g. the python
float would map to floating instead of float64 which it typically
maps to).
WarrenWeckesser pushed a commit that referenced this pull request Mar 22, 2020
This finishes the deprecation started in gh-9505 removing
behaviour that allowed strings/types representing specific dtypes
to behave like their more generic supertypes (e.g. the python
float would map to floating instead of float64 which it typically
maps to).

Co-Authored-By: Eric Wieser <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants