-
-
Notifications
You must be signed in to change notification settings - Fork 273
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
Fix doc #2556
Fix doc #2556
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2556 +/- ##
=======================================
Coverage 92.99% 92.99%
=======================================
Files 93 93
Lines 11086 11090 +4
=======================================
+ Hits 10309 10313 +4
Misses 777 777
Flags with carried forward coverage won't be shown. Click here to find out more.
|
astroid/nodes/node_classes.py
Outdated
@@ -2061,7 +2061,14 @@ def __init__( | |||
:param end_col_offset: The end column this node appears on in the | |||
source code. Note: This is after the last symbol. | |||
""" | |||
self.value: Any = value | |||
if getattr(value, "__name__", None) == "__doc__": | |||
raise AssertionError( # pragma: no cover |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this still needed now that you have fixed them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was tricky to debug, so I thought I might leave this as a help for future situations, as it's trivial to introduce this error. If you think it's unnecessary, I'll delete it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think if we want debugging we should make it a warning instead. It would be very annoying for users to have pylint
crash just because we introduced a small oopsie somewhere. Before we release both astroid
and pylint
we're often a couple weeks further so I think the AssertionError
is too much here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, replaced with a warning
ed927cb
to
f83d1cd
Compare
Head branch was pushed to by a user without write access
25b9f81
to
dfa52f1
Compare
(the linter's complained there's no "stacklevel" argument in the warning. I've fixed that) |
some '__doc__' fields of standard library symbols (e.g. WrapperDescriptorType.__doc__) don't return a string, they return a 'getset_descriptor'. Thus, an attempt to print "as string" fails. The solution is to check that __doc__ is an instance of str. Note that it wasn't uncovered by the tests due to classes not being attached to their parent in some cases. This is be done in one of the subsequent commits. it's a part of the campaign to get rid of non-module roots
it's a part of the campaign to get rid of non-module roots
dfa52f1
to
188b5d9
Compare
A part of getting rid of non-Module roots, see #2536