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 brain dict regression #2204

Merged

Conversation

knedlsepp
Copy link
Contributor

@knedlsepp knedlsepp commented Jun 7, 2023

Type of Changes

This fixes issue #2200, which should fix pylint-dev/pylint#7433.

Type
🐛 Bug fix

Description

Closes #2200

@knedlsepp knedlsepp marked this pull request as draft June 7, 2023 13:36
@knedlsepp
Copy link
Contributor Author

Put back as draft as this apparently breaks tests.

@knedlsepp
Copy link
Contributor Author

@jacobtylerwalls I tried to implement your suggestion but with that I apparently run into other test failures. I could get pylint working again with:

--- a/astroid/manager.py
+++ b/astroid/manager.py
@@ -66,6 +66,7 @@ class AstroidManager:
 
     def __init__(self) -> None:
         # NOTE: cache entries are added by the [re]builder
+        self.__dict__ = AstroidManager.brain
         self.astroid_cache = AstroidManager.brain["astroid_cache"]
         self._mod_file_cache = AstroidManager.brain["_mod_file_cache"]
         self._failed_import_hooks = AstroidManager.brain["_failed_import_hooks"]

At the moment I'm stuck as I don't know enough about astroid to continue here.

@jacobtylerwalls
Copy link
Member

Interesting. What happens if you revert the changes relating to the mutable values like {} and just introduce properties for manipulating the immutable ones like the booleans?

@knedlsepp knedlsepp force-pushed the fix-brain-dict-regression branch 2 times, most recently from 0d68276 to 1b3452a Compare June 7, 2023 14:34
@jacobtylerwalls jacobtylerwalls added this to the 2.15.6 milestone Jun 7, 2023
@knedlsepp knedlsepp marked this pull request as ready for review June 7, 2023 14:43
jacobtylerwalls
jacobtylerwalls previously approved these changes Jun 7, 2023
Copy link
Member

@jacobtylerwalls jacobtylerwalls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for identifying the issue and sending a patch. Welcome aboard!

tests/test_regrtest.py Outdated Show resolved Hide resolved
tests/test_regrtest.py Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jun 7, 2023

Codecov Report

Merging #2204 (6c105ad) into main (a6eb2b8) will increase coverage by 0.15%.
The diff coverage is 91.66%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2204      +/-   ##
==========================================
+ Coverage   92.53%   92.68%   +0.15%     
==========================================
  Files          94       94              
  Lines       10818    10830      +12     
==========================================
+ Hits        10010    10038      +28     
+ Misses        808      792      -16     
Flag Coverage Δ
linux 92.44% <91.66%> (+0.05%) ⬆️
pypy 87.64% <91.66%> (?)
windows 92.28% <91.66%> (+0.05%) ⬆️

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

Impacted Files Coverage Δ
astroid/manager.py 87.86% <91.66%> (+0.09%) ⬆️

... and 6 files with indirect coverage changes

jacobtylerwalls
jacobtylerwalls previously approved these changes Jun 7, 2023
@jacobtylerwalls jacobtylerwalls enabled auto-merge (squash) June 7, 2023 15:02
@knedlsepp knedlsepp force-pushed the fix-brain-dict-regression branch 2 times, most recently from cc2ccfb to 6ad6d2e Compare June 7, 2023 15:11
We get rid of the immutable instance attributes in AstroidManager,
ensuring that these always mutate the global state instead of
instance's.
This fixes a regression introduced in commit
bbcc58b.

Fixes pylint-dev#2200
jacobtylerwalls
jacobtylerwalls previously approved these changes Jun 7, 2023
@DanielNoord
Copy link
Collaborator

@jacobtylerwalls Do you want to add typing to these signatures?

@jacobtylerwalls
Copy link
Member

Nah, doesn't need to be done now. Doesn't provide much value anyway.

@jacobtylerwalls
Copy link
Member

The missing coverage is worth adding, though!

@jacobtylerwalls jacobtylerwalls merged commit b08166b into pylint-dev:main Jun 8, 2023
17 of 18 checks passed
github-actions bot pushed a commit that referenced this pull request Jun 8, 2023
Fix regression resulting in ignored pylint settings

We get rid of the immutable instance attributes in AstroidManager,
ensuring that these always mutate the global state instead of
instance's.
This fixes a regression introduced in commit
bbcc58b.

Fixes #2200

(cherry picked from commit b08166b)
jacobtylerwalls pushed a commit that referenced this pull request Jun 8, 2023
Fix regression resulting in ignored pylint settings

We get rid of the immutable instance attributes in AstroidManager,
ensuring that these always mutate the global state instead of
instance's.
This fixes a regression introduced in commit
bbcc58b.

Fixes #2200

(cherry picked from commit b08166b)

Co-authored-by: Josef Kemetmüller <josef.kemetmueller@gmail.com>
@knedlsepp
Copy link
Contributor Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported Assigned once the backport is done Bug 🪳
Projects
None yet
4 participants