Skip to content

Propose a less error-prone helper for module-level getattrs. #20857

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

Merged
merged 2 commits into from
Aug 20, 2021

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Aug 18, 2021

Apparently we always forget to generate the fallback AttributeError (#20822, #20855), so
introduce a helper to do so instead. (I'm not wedded to the exact API...)

The changes in deprecation.py are (a) so that one can correctly
suppress the inferred type when deprecating properties (the new check is
coherent with the if isinstance(obj, type) and other cases) and (b) to
make single-argument __get__ work on deprecated properties
(consistently with standard properties).

Would replace #20856.

PR Summary

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

@lpsinger
Copy link
Contributor

Does this method correctly distinguish between __getattr__ returning None for an attribute that is intended to exist but be defined as None, versus __getattr__ raising an exception for an attribute that does not exist?

@anntzer
Copy link
Contributor Author

anntzer commented Aug 18, 2021

Yes. (In fact, that's the reason why I switched to a class-based approach, as a function-based approach would have trouble distinguishing either between missing globals and globals set to None, or between genuine AttributeErrors and AttributeErrors arising because of a bug in the calculation of the dynamically generated value.)

@anntzer anntzer force-pushed the module-getattr branch 2 times, most recently from 5d30b57 to 87392d8 Compare August 18, 2021 22:00
@QuLogic QuLogic added this to the v3.5.0 milestone Aug 18, 2021
@QuLogic QuLogic added Maintenance Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. status: needs rebase labels Aug 19, 2021
anntzer and others added 2 commits August 20, 2021 16:14
Apparently we always forget to generate the fallback AttributeError, so
introduce a helper to do so instead.

The changes in `deprecation.py` are (a) so that one can correctly
suppress the inferred type when deprecating properties (the new check is
coherent with the `if isinstance(obj, type)` and other cases) and (b) to
make single-argument `__get__` work on deprecated properties
(consistently with standard properties).
@tacaswell
Copy link
Member

I also cherry-picked @lpsinger 's commit from #20856 into this PR just to be sure that they did not conflict with each other.

@lpsinger
Copy link
Contributor

Would replace #20856.

I don't know that it replaces it --- that PR simply adds a unit test.

@QuLogic
Copy link
Member

QuLogic commented Aug 20, 2021

Would replace #20856.

I don't know that it replaces it --- that PR simply adds a unit test.

But it does now:

I also cherry-picked @lpsinger 's commit from #20856 into this PR just to be sure that they did not conflict with each other.

@QuLogic QuLogic merged commit af68218 into matplotlib:master Aug 20, 2021
@anntzer anntzer deleted the module-getattr branch August 21, 2021 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Maintenance Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants