Title: Limit Forward references in Mathtext parser by ksunden · Pull Request #26198 · matplotlib/matplotlib · GitHub
Open Graph Title: Limit Forward references in Mathtext parser by ksunden · Pull Request #26198 · matplotlib/matplotlib
X Title: Limit Forward references in Mathtext parser by ksunden · Pull Request #26198 · matplotlib/matplotlib
Description: PR summary Comment indicates that limiting forward references is important for performance, but many more were used than needed. This is a 75% reduction in forward reference instances. Many of these are included in the definition of remaining forward references but do not themselves need to be a forward reference (indeed only 1 thing in a given cycle actually needs to be such that it can be referred to prior to its definition) auto_delim is recursively self-referential, and thus must be all on its own placeable and accent form a tight loop, so one of those two needed to be a forward reference, and placeable was the easiest to keep (and most likely to be used in another definition that may introduce similar constraints) token is one of the more generic definitions, that is used in multiple other definitions. required_group and optional_group both caused failed tests when I tried to make them no longer forward references, has something to do with how __call__ works for Forward vs And objects in pyparsing (though did not dig too much deeper into it, beyond noticing that the names in future required_group("name") calls still had error messages that say "group", not their newer names, and that reverting to Forward instances fixed it). operatorname needed to move to avoid using forward references for simple, but does not actually have a cycle. Otherwise rewrapped some of the definitions to be one line and fixed a typing error that I noticed when looking into the parsing things. PR checklist [N/A] "closes #0000" is in the body of the PR description to link the related issue [N/A] new and changed code is tested [N/A] Plotting related features are demonstrated in an example [N/A] New Features and API Changes are noted with a directive and release note [N/A] Documentation complies with general and docstring guidelines @devRD I would appreciate your review on this. If this reorg would cause conflicts for other PRs already in flight, it can absolutely wait. I just saw the comment saying to minimize usage of Forward and then was immediately like "why is this a Forward" and kept going. The changes themselves are relatively minimal and would be easy to redo if needed: delete line that instantiates the Forward delete the << from <<= in the redefinition a few reorderings to maintain directionality without Forword references keep anything that is referred to before it is defined (and cannot be reordered) as a Forward (plus the optional/required_group)
Open Graph Description: PR summary Comment indicates that limiting forward references is important for performance, but many more were used than needed. This is a 75% reduction in forward reference instances. Many of thes...
X Description: PR summary Comment indicates that limiting forward references is important for performance, but many more were used than needed. This is a 75% reduction in forward reference instances. Many of thes...
Opengraph URL: https://github.com/matplotlib/matplotlib/pull/26198
X: @github
Domain: github.com
| route-pattern | /:user_id/:repository/pull/:id/files/:range(.:format) |
| route-controller | pull_requests |
| route-action | files |
| fetch-nonce | v2:3ac68cbf-860b-b82a-02b4-2014e6cdfe1b |
| current-catalog-service-hash | ae870bc5e265a340912cde392f23dad3671a0a881730ffdadd82f2f57d81641b |
| request-id | A1B2:1A138C:2068F56:2C0A71C:6A65559B |
| html-safe-nonce | 9d504319fb385cf72ffd5c8fad6c9eeeeca64241c05ebe82ee764f2e69e9e7be |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiJBMUIyOjFBMTM4QzoyMDY4RjU2OjJDMEE3MUM6NkE2NTU1OUIiLCJ2aXNpdG9yX2lkIjoiNzE0MTc0NjE5ODI3MTMxODAzIiwicmVnaW9uX2VkZ2UiOiJpYWQiLCJyZWdpb25fcmVuZGVyIjoiaWFkIn0= |
| visitor-hmac | bc24f0b1829b28ed943c4dab6968c53db2ad5a364f7bd39a29312cb7cfe8630b |
| hovercard-subject-tag | pull_request:1409059415 |
| 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/26198/files/44ee3d62ea5003b143cf2742b71272beb66387e6 |
| twitter:image | https://avatars.githubusercontent.com/u/2501846?s=400&v=4 |
| twitter:card | summary_large_image |
| og:image | https://avatars.githubusercontent.com/u/2501846?s=400&v=4 |
| og:image:alt | PR summary Comment indicates that limiting forward references is important for performance, but many more were used than needed. This is a 75% reduction in forward reference instances. Many of thes... |
| og:site_name | GitHub |
| og:type | object |
| hostname | github.com |
| expected-hostname | github.com |
| None | 52c76df668885aaff23b50bdca1fa1ea44ac9c1553e888ebc70ff1e4daa4625b |
| 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 full-width |
| disable-turbo | true |
| browser-stats-url | https://api.github.com/_private/browser/stats |
| browser-errors-url | https://api.github.com/_private/browser/errors |
| release | 309153364422b3c499922d1a2a6404910a58ed8e |
| ui-target | full |
| theme-color | #1e2327 |
| color-scheme | light dark |
Links:
Viewport: width=device-width