Title: moved cla shared axes code to a dedicated function by jrmlhermitte · Pull Request #8710 · matplotlib/matplotlib · GitHub
Open Graph Title: moved cla shared axes code to a dedicated function by jrmlhermitte · Pull Request #8710 · matplotlib/matplotlib
X Title: moved cla shared axes code to a dedicated function by jrmlhermitte · Pull Request #8710 · matplotlib/matplotlib
Description: PR Summary Response to the end of #8455 with discussion with @WeatherGod The goal of this PR is to clean up the code in Axes.cla(), which has grown to be large. We probably want something clean over something quick here. This is meant to be a very simple aesthetic change, nothing more (no logic changes). PR Checklist Has Pytest style unit tests Code is PEP 8 compliant New features are documented, with examples if plot related Documentation is sphinx and numpydoc compliant Added an entry to doc/users/whats_new.rst if major new feature Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way PR Details This is also just a suggestion, and there likely may be a better way. I am opening it so it does not get lost. I am open to suggestions, and if you have something nicer, feel free to create a new PR (but please reference this one so we don't get lost). However, I believe it does bring some points of discussion that I think may be necessary to address, such as: I think I may be breaking convention (from what I see in code) by having the function take a string and use it to call class methods, but so far it seems to be a cleaner and still readable to me. If cla is overridden, then _cla_shared needs to be also removed from the class (i.e. for polar axes). Right now this would have to be manually done. I am wondering if there exist decorators implemented that take care of these? For example: def MyNewClass(OldClass): @cla.override def cla(self): #.... where @cla.override would contain the necessary bookeeping to remove _cla_shared when cla is overridden. I will try to search more when I have time but if someone has insight I'm interested to know (and thanks for saving me time :-) ). @QuLogic might have some idea and some input with his experience with polaraxes. Will need to wait for Travis. My local tests on test/test_axes.py contain a long list of failures (which seem to be the same with and without this change). thanks!
Open Graph Description: PR Summary Response to the end of #8455 with discussion with @WeatherGod The goal of this PR is to clean up the code in Axes.cla(), which has grown to be large. We probably want something clean ove...
X Description: PR Summary Response to the end of #8455 with discussion with @WeatherGod The goal of this PR is to clean up the code in Axes.cla(), which has grown to be large. We probably want something clean ove...
Opengraph URL: https://github.com/matplotlib/matplotlib/pull/8710
X: @github
Domain: github.com
| route-pattern | /:user_id/:repository/pull/:id/files(.:format) |
| route-controller | pull_requests |
| route-action | files |
| fetch-nonce | v2:97e13943-bac8-aa73-091b-7e3175ee15b6 |
| current-catalog-service-hash | ae870bc5e265a340912cde392f23dad3671a0a881730ffdadd82f2f57d81641b |
| request-id | 856C:2F0FE8:CB0797:10C9EB7:6A53263E |
| html-safe-nonce | fad7d687aebdd08287ccc65b259018429a54897c1a052f46e1edb866f95eddaa |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiI4NTZDOjJGMEZFODpDQjA3OTc6MTBDOUVCNzo2QTUzMjYzRSIsInZpc2l0b3JfaWQiOiI3NjQ1MjA1NDg1OTIxOTkyMzAiLCJyZWdpb25fZWRnZSI6ImlhZCIsInJlZ2lvbl9yZW5kZXIiOiJpYWQifQ== |
| visitor-hmac | bf176c2f862fd5d971c95eaf5cb0cbbff34848584c504aad17b7244d5e4e81bd |
| hovercard-subject-tag | pull_request:123848913 |
| github-keyboard-shortcuts | repository,pull-request-list,pull-request-conversation,pull-request-files-changed,copilot |
| google-site-verification | Apib7-x98H0j5cPqHWwSMm6dNU4GmODRoqxLiDzdx9I |
| octolytics-url | https://collector.github.com/github/collect |
| analytics-location | / |
| fb:app_id | 1401488693436528 |
| apple-itunes-app | app-id=1477376905, app-argument=https://github.com/matplotlib/matplotlib/pull/8710/files |
| twitter:image | https://avatars.githubusercontent.com/u/705366?s=400&v=4 |
| twitter:card | summary_large_image |
| og:image | https://avatars.githubusercontent.com/u/705366?s=400&v=4 |
| og:image:alt | PR Summary Response to the end of #8455 with discussion with @WeatherGod The goal of this PR is to clean up the code in Axes.cla(), which has grown to be large. We probably want something clean ove... |
| og:site_name | GitHub |
| og:type | object |
| hostname | github.com |
| expected-hostname | github.com |
| None | b9a586c06a05a7a86fc7e3f4dbd03e42f6869085879aa184aa6369456dbd50fb |
| turbo-cache-control | no-preview |
| diff-view | unified |
| go-import | github.com/matplotlib/matplotlib git https://github.com/matplotlib/matplotlib.git |
| octolytics-dimension-user_id | 215947 |
| octolytics-dimension-user_login | matplotlib |
| octolytics-dimension-repository_id | 1385122 |
| octolytics-dimension-repository_nwo | matplotlib/matplotlib |
| octolytics-dimension-repository_public | true |
| octolytics-dimension-repository_is_fork | false |
| octolytics-dimension-repository_network_root_id | 1385122 |
| octolytics-dimension-repository_network_root_nwo | matplotlib/matplotlib |
| turbo-body-classes | logged-out env-production page-responsive |
| disable-turbo | true |
| browser-stats-url | https://api.github.com/_private/browser/stats |
| browser-errors-url | https://api.github.com/_private/browser/errors |
| release | 07a982c1d40157c619b364352b704c3ce66bb332 |
| ui-target | full |
| theme-color | #1e2327 |
| color-scheme | light dark |
Links:
Viewport: width=device-width