Comments (18)
Is there a way to disable these really annoying warnings? We have some legacy code that we don't want to update but we still have to maintain it for a while.
Why would we have a warning that cannot be disabled using the normal ignore mechanism?
from pydocstyle.
This is because if __all__
is mutable, then pep257 can't really tell if it was modified or not. So I decided to stick to tuples. Didn't think it would be a problem. Is it?
from pydocstyle.
Most of the code I've seen makes __all__
a list, as do all of the examples in the official Python documentation. PEP 8 also refers to __all__
as a list. And there may be other common tools that rely on this convention (although that would technically be incorrect).
Also making __all__
a tuple doesn't completely address the problem you're trying to solve. While the tuple is immutable, the code can change the binding of __all__
to reference a different tuple.
Perhaps a good approach is to skip the __all__
validation if it is a list, or make it a command line option to do so.
from pydocstyle.
Also making
__all__
a tuple doesn't completely address the problem you're trying to solve. While the tuple is immutable, the code can change the binding of__all__
to reference a different tuple.
__all__
could be only re-bound at the top level. pep257 will recognise that.
Perhaps a good approach is to skip the
__all__
validation if it is a list, or make it a command line option to do so.
That would defeat the purpose of having __all__
. pep257 would not know what's public and what's not.
from pydocstyle.
I'm in complete agreement that making __all__
a tuple is a win for static checking. My only point in creating this issue was to point out that in the bulk of python code __all__
is a list so most users of pep257 will see this error message.
That said I'll close this issue. Perhaps generating this message will provide incentive to switch to using tuples to get the benefit of __all__
checking.
BTW thanks for creating this handy tool -- I run all my python code through it to keep myself consistent.
from pydocstyle.
Is there a need for pep257 to know what is public or not? Are there different PEP-257 rules for public and private things?
from pydocstyle.
@ziadsawalha PEP 257 says:
...and all functions and classes exported by a module should also have docstrings.
To enforce that pep257 should know which functions and classes are private and which are public. If there is no __all__
it is simple—things with leading underscore are private, otherwise—public. With __all__
it is slightly more difficult. If __all__
is mutable, then there is no way to determine it contents statically. Maybe the module modifies __all__
based on current temperature in Rome. However, if it is immutable (tuple), we can reason about what is public and what is private statically.
from pydocstyle.
So pep257 checks that __all__
is a tuple not for its own sake, but to give warnings about public functions which don't have docstrings.
from pydocstyle.
It's probably best to document this behavior either in README or in the error-message, though.
from pydocstyle.
FWIW I’ve seen __all__
being a list 99% of the time. I think I remember a Python bug where inspect or pydoc choked on a tuple __all__
, which shows that Python core developers always thought it would be a list.
from pydocstyle.
@halst Could you help me find a test or condition under which #63 will fail? I'll be happy to modify the code around that.
from pydocstyle.
__all__ = ['g']
def f():
pass
def g():
"""Docstring."""
__all__.append('f')
In this case, even though f
is in __all__
, pep257 will not be able to warn that f
is missing a docstring, because it is public.
from pydocstyle.
Yeah..... hmmmm
But that's almost impossible to get water-tight. What if you have something like this:
import os
__all__ = ['g']
def f():
pass
def g():
"""Docstring."""
if os.environ.get("FEATURE_F"):
__all__.append('f')
I feel like I've come across many more projects that have all as a static list than I have come across ones where it is dynamically changed; but that might be the types of projects I work with. I don't have solid data on that and I'm not sure how to get it.
How about if we add an option (ex. --all-as-list
) that made pep257 not break on all as a list?
from pydocstyle.
I would rather have no option, but a warning if __all__
is a list. Something along the lines of:
./file.py: WARNING:
__all__
is defined as a list, this means pep257 cannot reliably detect contents of__all__
variable, because it can be mutated. Change__all__
to be an (immutable) tuple, to remove this warning. Note, pep257 is using__all__
to detect which definitions are public, to warn if public definitions are missing docstrings. If__all__
is a (mutable) list, pep257 cannot reliably assume its contents. Now pep257 will proceed assuming__all__
is not mutated.
Not sure that this is the best wording, though. Anyway, pep257 will proceed and will return 0 code in no other errors were found—so it's a pure warning that will not influence success/failure of the static analysis.
from pydocstyle.
@halst How about this? I added the following to #63:
if self.current.value == '[':
msg = ("%s WARNING: __all__ is defined as a list, this means "
"pep257 cannot reliably detect contents of the __all__ "
"variable, because it can be mutated. Change __all__ to be "
"an (immutable) tuple, to remove this warning. Note, "
"pep257 uses __all__ to detect which definitions are "
"public, to warn if public definitions are missing "
"docstrings. If __all__ is a (mutable) list, pep257 cannot "
"reliably assume its contents. pep257 will proceed "
"assuming __all__ is not mutated.\n" % self.filename)
sys.stderr.write(msg)
And the output looks like this:
$pep257 pep257.py | grep WARN
pep257.py WARNING: __all__ is defined as a list, this means pep257 cannot reliably detect
contents of the __all__ variable, because it can be mutated. Change __all__ to be an (immutable)
tuple, to remove this warning. Note, pep257 uses __all__ to detect which definitions are public, to
warn if public definitions are missing docstrings. If __all__ is a (mutable) list, pep257 cannot
reliably assume its contents. pep257 will proceed assuming __all__ is not mutated.
from pydocstyle.
Thanks, @halst. @smoyergh - this is now addressed in #63 and merged in.
from pydocstyle.
I'm not sure I see the point of this warning, as even __all__
as a tuple can be modified:
import os
__all__ = ('g',)
def f():
pass
def g():
"""Docstring."""
if os.environ.get('FEATURE_F'):
__all__ += ('f',)
This makes __all__
point to a new tuple object in memory, so it's not actually mutable, but both lists and tuples have the same problem when statically parsing a module.
from pydocstyle.
The Python official docs refer to __all__
as a list and write all examples using a list. This warning is contradicting the language's own documentation. Further, as @ask has shown, this offers no real protection against mutating the __all__
reference.
IMO, I think you should consider removing thewarning and instead document pydocstyle's shortfall when handling modules that mutate __all__
.
from pydocstyle.
Related Issues (20)
- option to ban `noqa` comments that don't specify a code
- D102 - false positive on overload with comment
- cosmetic bug: wrong version stamp in webpage title: 1.0.0 instead of 2.1.1
- The match-dir parameter is not used. HOT 4
- Not detecting wrong DocStrings HOT 1
- Your website is down - https://pydocstyle.org HOT 3
- Crash with simple f-string HOT 7
- Docs website is down? HOT 1
- sdist: PKG-INFO should list LICENSE-MIT in License-File
- `D300` and `D301` false positives on docstrings with escaped triple quotes in them
- Should `__main__.py` really be considered a public module?
- Option to not report error for empty file HOT 1
- test_simple_fstring and test_fstring_with_args fail with Python 3.12 beta HOT 2
- 6.3.0: incorrect version in pyproject.toml?🤔 HOT 2
- pydocstyle 6.3.0 fails silently on a python script that has no .py extension
- Rule for missing PEP 698 `@overrides` decorators?
- Keyword Args not recognized in Google convention with no return
- Python3.12: Two tests fail HOT 1
- Google convention section names not properly checked
- D417 not raised in a standard Google configuration with missing arguments descriptions
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from pydocstyle.