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

Bump pyasn1 to 0.6.* #11681

Merged
merged 2 commits into from
Apr 1, 2024
Merged

Bump pyasn1 to 0.6.* #11681

merged 2 commits into from
Apr 1, 2024

Conversation

Avasam
Copy link
Collaborator

@Avasam Avasam commented Mar 31, 2024

Closes #11664

None of these change were detected by stubtest because:

  • Names are omitted from __all__ (see comment below)
  • RealEncoder.supportIndefLenMode: bool being a subclass of int, this technically passed.

@AlexWaygood
Copy link
Member

AlexWaygood commented Mar 31, 2024

Should we be adding symbols if they're not included in __all__? The reason why stubtest doesn't complain about those being missing is that it assumes stub authors won't want to add things to stubs that aren't public API. And if it's missing from __all__, it's been explicitly marked as not being public API.

(If there's a specific reason to add any of these symbols, or we think they were just accidentally omitted from __all__, then it's obviously fine to add them — I just don't want us doing it just for "completeness"!)

This comment has been minimized.

@Avasam
Copy link
Collaborator Author

Avasam commented Mar 31, 2024

Maybe? but as far as I can tell, nearly all symbols from pyasn1 are currently exposed, so I went with established precedent for this stub.
These were really not complex to add (a quick merge from stubgen), I don't mind reverting it and only keeping the int --> bool change.

As a sidenote, these stubs are really simple and quite incomplete as far as annotations go. 6.0 just dropped support for Python 2.7 and 3.7. If the pyasn1 maintainers are open to it, it shouldn't be too hard to merge these stubs upstream.

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Thanks! I don't feel too strongly about the symbols that aren't included in __all__, but I do think it's good if we think about why we're adding symbols marked as private API whenever we do so (it's much harder to remove something from a stub than it is to add it, due to backwards compatibility!)

Copy link
Contributor

github-actions bot commented Apr 1, 2024

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@AlexWaygood AlexWaygood merged commit d72a34a into python:main Apr 1, 2024
43 checks passed
@Avasam Avasam deleted the Bump-pyasn1-to-0.6.- branch April 1, 2024 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants