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
configparser.__all__ is incomplete #71293
Comments
That's a child issue of bpo-23883, created to propose a patch fixing configparser module's __all__ list. |
Looks good to me |
The reason we specifically omitted Error was two-fold:
So I'm torn a little here. On the one hand, it's nice to add Error for completeness. On the other hand, is this change solving a real issue or just satisfying your inner librarian? The reason we have to ask ourselves this question is that this change bears a small risk of breaking user code that was working before. Take a look at this example:
Sure, it's bad code but the point is: it was working before and had a decent error handling strategy. With the change in __all__, it will just crash because wave.Error was never caught. Is this likely to happen? I don't think so. Knowing my luck, will it happen to somebody? Yeah. So the question remains: why do we want Error in __all__ in the first place? Is it worth it? |
My personal opinion is to include all public APIs. Names that are omitted from __all__ may not come up in pydoc, and it is surprising when I use “import * ” in the interactive interpreter to play with a module and there is something missing. To mitigate the risk of breaking code, I have been maintaining a list of the modules affected at <https://docs.python.org/3.6/whatsnew/3.6.html#changes-in-the-python-api\>, which warns that extra symbols will be imported in 3.6. On the other hand, there are other cases where people wanted to exclude APIs from __all__; I pointed out two at <https://bugs.python.org/issue26632#msg266117\>. |
Łukasz, Martin - I'm not sure how to proceed here, of course both ways out are reasonable. I'd be happy to provide (however small) patch for either one (adding Error to __all__ or just adding test for __all__). :) My inner librarian would of course either push the Error to fully become part of public API or change its name to non-public _Error or something, but I'd much rather rely on you in this regard. |
I'm still not convinced this change is *useful*. The list of incompatibilities in Python 3.6 in whatsnew is going to be used retrospectively by people already affected by a production issue, googling for an explanation as to what went wrong. I like the idea of adding a unit test that clarifies the omission is deliberate at this point. |
That's completely fine for me. I'm attaching the patch that just adds test for __all__, then. :) |
New changeset 739528288996 by Martin Panter in branch 'default': |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: