10000 multiple inheritance of static C api types is not supported by CPython, yet we do it anyway · Issue #11998 · numpy/numpy · GitHub
[go: up one dir, main page]

Skip to content

multiple inheritance of static C api types is not supported by CPython, yet we do it anyway #11998

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

Open
eric-wieser opened this issue Sep 20, 2018 · 15 comments

Comments

@eric-wieser
Copy link
Member
eric-wieser commented Sep 20, 2018

Our scalar type classes are such that:

  • issubclass(np.int_, int) and issubclass(np.int_, np.integer) (py2 only)
  • issubclass(np.float_, float) and issubclass(np.float_, np.floating)
  • issubclass(np.complex_, float) and issubclass(np.complex_, np.complexfloating)
  • ...

We do this in the C api by manually copying across some tp_* slots, setting tp_bases to a tuple, and crossing our fingers. However, the CPython API docs say (emphasis mine):

PyTypeObject* PyTypeObject.tp_base

An optional pointer to a base type from which type properties are inherited. At this level, only single inheritance is supported; multiple inheritances require dynamically creating a type object by calling the metatype.

PyObject* PyTypeObject.tp_bases

Tuple of base types.

This is set for types created by a class statement. It should be NULL for statically defined types.

We do not do this, so it seems we're relying on internal behavior we're not supposed to.

@eric-wieser
Copy link
Member Author

CPython becomes aware of our hack if I try to add a metaclass to 'np.generic', and complains that 'np.generic' and 'float' have mismatching layouts

@shoyer
Copy link
Member
shoyer commented Oct 15, 2018

I would be in favor of removing all inheritance from Python types as part of a dtype refactor/clean-up. Is there any way we could do a deprecation cycle for this?

@tylerjereddy
Copy link
Contributor

I would be in favor of removing all inheritance from Python types as part of a dtype refactor/clean-up. Is there any way we could do a deprecation cycle for this?

At the risk of straying a little off-topic, it would be really useful to define discrete engineering tasks related to the proposed dtype overhaul that the core devs think would be useful (even as experiments likely to fail).

Perhaps this is one such engineering task. I know there are some broader documents & discussions floating around.

@tylerjereddy
Copy link
Contributor

@eric-wieser Could you provide example code that detects the issue?

@eric-wieser
Copy link
Member Author

I have a patch somewhere that brings it up - will make a PR out of it for the sake of producing CI logs

@teoliphant
Copy link
Member
teoliphant commented Oct 27, 2018

The Python C-API docs were changed. Multiple inheritance was allowed 12 years ago and was not discouraged like it is now. The only requirement was that the binary ABIs were compatible --- which they did. This is re-writing history to claim otherwise.

We did not "cross-our fingers" . This was supported behavior that was changed in Python and NumPy did not update because I would be the only one who knew how and why this was implemented and have not been involved due to lack of time and funding.

It's a maintenance issue, and a movement of Python issue. This should now be cleaned up, yes, as Python has changed its stance in Python 3.

This was carefully considered and implemented to ensure NumPy floats behaved like Python floats as much as possible.

@teoliphant
Copy link
Member

I would be in favor of removing all inheritance from Python types as part of a dtype refactor/clean-up. Is there any way we could do a deprecation cycle for this?

This is an API change that should only really happen in a major release. It is a real semantic change that should be signaled by a major version bump.

@charris
Copy link
Member
charris commented Oct 27, 2018

@teoliphant We are unlikely to have a major release anytime soon, maybe never. Apart from whether or not it was good python practice in the past, the double inheritance makes NumPy float64s oddballs in the NumPy float universe. It was a wonderful thing when NumPy ints no longer inherited from python ints when Python 3 came along and I think it would be nice to extend that to float64.

@charris
Copy link
Member
charris commented Oct 27, 2018

to ensure NumPy floats behaved like Python floats as much as possible.

Which is exactly the problem and why it would be good to change it.

@teoliphant
Copy link
Member

Obviously, I think we should not fear the major version number bump. Even if the major version is to publish a "minimum API for NumPy" and not do major code re-writes. In other words, there is a minimum concept of a NumPy 2.0 that does not take unbounded effort. That should be the start. There are enough of these little problems to fix that are not hard to get a large majority of supporters for.

@teoliphant
Copy link
Member

As it stands, removing double inheritance is relatively minor compared to other changes that have happened in .x release (i.e. NumPy doesn't really adhere to semantic versioning). I cannot immediately think of problems it would cause. And if it did, those can be easily fixed.

@charris
Copy link
Member
charris commented Oct 27, 2018

Sure, but why make the change if it means little? Linux hasn't had a " 8000 real" new version release since 2.6.0, and the only reason the version/major numbers change were that Linus thought the release numbers were getting too big. We aren't there yet...

I suppose we could pick an arbitrary number, say 25, and release NumPy 2.0.0 instead of 1.26.0, just because, but the current thinking is to just evolve, which means steady change rather than jumps. I think that the Python 3 experience, as well as the several failed NumPy 2 attempts, have led to that approach.

@charris
Copy link
Member
charris commented Oct 27, 2018

That said, I can see jumping the major release number for marketing reasons, so to speak. NumPy has come a long way since 1.0, new people are doing the development, the governance and organization are changing, and it might be nice to recognize all that by going to 2.0.

@rgommers
Copy link
Member

That said, I can see jumping the major release number for marketing reasons, so to speak. NumPy has come a long way since 1.0, new people are doing the development, the governance and organization are changing, and it might be nice to recognize all that by going to 2.0.

That is possibly a good idea. We should then not tie it to some backwards-incompatible change though, we're pretty much all agreed indeed that gradual evolution and basing deprecations based on "pain for users" is the way to go. 2.0 could mark some random point in time (for marketing reasons), or for example the completion of a major feature like __array_function__.

@eric-wieser
Copy link
Member Author

This is re-writing history to claim otherwise.

That was not my intent - thanks for delivering the correct history!

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

No branches or pull requests

6 participants
0