Proper parsing of imports to avoid misunderstanding __all__ imports #187

Merged
merged 5 commits into from Jun 18, 2016

Projects

None yet

2 participants

@Nurdok
Member
Nurdok commented May 11, 2016 edited

Fixes #182

@jayvdb
Contributor
jayvdb commented May 12, 2016

I think it would be useful to have a test case for from x import __all__ without an as alias.

see here for a simple and reasonable example of that by @Suor:
https://github.com/Suor/funcy/blob/master/funcy/__init__.py
Not a particularly great example for this PR, as it is best case since it doesnt modify __all__ after that import, which is where the problems start.

rules around modification of that imported __all__ are probably best reported (if at all) by pyflakes or pycodestyle, but pydocstyle should at least have a predictable, reasonable, and stable result for that obscure scenario also.

@Nurdok
Member
Nurdok commented Jun 3, 2016

@jayvdb Added. Can you take a look at the changes?

@jayvdb
Contributor
jayvdb commented Jun 6, 2016

Sorry for the delay.

When from x import __all__ is used, without an as, I think AllError needs to be raised.

A design aspect to consider is whether pydocstyle should allow code like https://raw.githubusercontent.com/Suor/funcy/master/funcy/__init__.py , as mentioned above. i.e. should AllError only be raised if there are any symbols actually defined in the module which might need docstrings? As that module doesnt have any possibly-public symbols, there is no need to add docstrings, irrespective of __all__ or not, and therefore no need to raise AllError. OTOH, it is bad style, and it meets the general requirements for AllError, so it is reasonable (and much easier) to raise the exception anyway. But a decision needs to be made regarding that, and test should ensure that the decision made now isn't reversed later accidentally with an inadvertent breaking change.

@Nurdok
Member
Nurdok commented Jun 18, 2016

I agree about raising the error when importing __all__, but the code structure makes it a pretty big change. I'll merge this for now and work on a more structural change.

@Nurdok Nurdok merged commit e1fd706 into PyCQA:master Jun 18, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment