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

add mapping & sequence type flags to the stable api #32080

Closed

Conversation

davidhewitt
Copy link
Contributor

@davidhewitt davidhewitt commented Mar 23, 2022

In #25723 (comment) it was implied that these flags should be "internal", but it's not entirely clear to me why. PEP 634 doesn't make a clear indication whether they should be in the stable API or not.

These flags are already readily available when compiling C extensions with the full API and can be used in two ways:

  • to implement native classes which can be used as mappings in match cases
  • to test for sequence / mapping types in the same way that the match keyword would

It would be nice to have them available in the stable ABI, if that's deemed reasonable.

@davidhewitt
Copy link
Contributor Author

Hi, is there someone available who would be willing to review this please? Maybe @markshannon as the original author of these flags?

@kumaraditya303
Copy link
Contributor

Please create an issue to discuss this change. Thanks

@TeamSpen210
Copy link

That comment doesn't look like it's describing the flags itself, but instead whether they should be present as public constants in collections.abc itself, which it shouldn't. Type flags aren't part of the language, just the C API.

Instead of using the flags, an alternative for the equivalent functionality would be to do the same way Python code does - import collections.abc.Mapping/Sequence, then call .register() or PyObject_IsInstance()/PyObject_IsSubclass(). Less performant of course, but it'll work on pretty much any Python version.

@markshannon
Copy link
Member

I don't think these flags should be part of the stable API.

As @TeamSpen210 says, using collections.abc.Sequence.register() is more robust, as it ensures consistency of flags and abstract base class.

@davidhewitt
Copy link
Contributor Author

Understood, thanks for the feedback - will close this 👍

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.

6 participants