Skip to content
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

Merged
merged 2 commits into from
Sep 11, 2024
Merged

Fix doc #2556

merged 2 commits into from
Sep 11, 2024

Conversation

temyurchenko
Copy link
Contributor

A part of getting rid of non-Module roots, see #2536

Copy link

codecov bot commented Sep 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.99%. Comparing base (e442776) to head (188b5d9).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2556   +/-   ##
=======================================
  Coverage   92.99%   92.99%           
=======================================
  Files          93       93           
  Lines       11086    11090    +4     
=======================================
+ Hits        10309    10313    +4     
  Misses        777      777           
Flag Coverage Δ
linux 92.87% <100.00%> (+<0.01%) ⬆️
pypy 92.99% <100.00%> (+<0.01%) ⬆️
windows 92.97% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
astroid/nodes/node_classes.py 94.94% <100.00%> (+<0.01%) ⬆️
astroid/nodes/scoped_nodes/scoped_nodes.py 92.80% <ø> (ø)
astroid/raw_building.py 94.98% <100.00%> (+0.04%) ⬆️

@@ -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
Copy link
Collaborator

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?

Copy link
Contributor Author

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

Copy link
Collaborator

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.

Copy link
Contributor Author

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

DanielNoord
DanielNoord previously approved these changes Sep 10, 2024
@DanielNoord DanielNoord enabled auto-merge (squash) September 10, 2024 19:37
auto-merge was automatically disabled September 10, 2024 19:49

Head branch was pushed to by a user without write access

@temyurchenko
Copy link
Contributor Author

(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
@DanielNoord DanielNoord merged commit 523eeb4 into pylint-dev:main Sep 11, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants