-
-
Notifications
You must be signed in to change notification settings - Fork 290
Limit error message overriding #493
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
Conversation
Gentle ping @ptmcg, would like to move this conversation forward, happy to do more if that is wanted |
I haven't forgotten. On vacation this week, will try to sneak in a few minutes during quiet time, otherwise, next week. |
This change looks like you have narrowed the field down to being detected by a minimal condition, thanks! I'll look at it in more detail in my dev environment when I get back home in a few days. Could you add some tests to test_unit.py for this please? Include some cases that are not strictly aligned with your usage (see how Forward is used elsewhere in the test cases or the examples). If you don't care to work with unittest (not everyone's favorite), just attach a small test file to this PR and I'll incorporate it into test_unit.py myself. |
I've added a test which is a significantly pared down version of what we do in matplotlib (though does use the same mechanism for raising the more custom exception) I do check as well, though, that if you set the name that the more default behavior returns. Not fully sure what other test cases would be warranted, as this does trace both branches of the introduced conditional. |
Gentle nudge on this one, we are preparing for a release and would like to fix up (or remove) pins on pyparsing/xfail tests based on the specific version of pyparsing (as everything works other than having less helpful error messages, so may not want to wholly ban mpl from installing with pyparsing 3.1.0) |
Released today in pyparsing 3.1.1 |
Thanks, @ptmcg! |
I've added matplotlib-specific tests to the pyparsing test suite, cloned from your tests: https://github.com/pyparsing/pyparsing/blob/master/tests/test_matplotlib_cases.py |
This is a continuation from the conversation in matplotlib/matplotlib#26152.
I'm opening this here to further the conversation and so we can talk in slightly more concrete terms.
This is based on my last suggestion in that thread where I suggested Forward references without a name do not override the error message.
Either one of these factors or another error message override flag would be sufficient for matplotlib's purposes, so if you feel that this could be broader or would prefer to introduce a more explicit test, then feel free to say so and I will try to do so.