-
Notifications
You must be signed in to change notification settings - Fork 2
Reordered operations in Map.add_vertex #197
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
Conversation
Fix is made to avoid hitting a race condition due to weakref.finalize() being called. It seems that .finalize temporarily deletes references of the object passed to it from dicts, therefore trying to get it from __type_dict or _store might randomly raise a KeyError.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## more_weakref_fixes #197 +/- ##
===================================================
Coverage 81.15% 81.16%
===================================================
Files 52 52
Lines 4267 4268 +1
Branches 740 740
===================================================
+ Hits 3463 3464 +1
Misses 624 624
Partials 180 180
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
How is this fixing the issue? Can you explain what those lines are changing? |
Just for reference, the traceback that lead to the creation of this PR Weird thing is that here we see sequential Here is my assumption of what is happening: Basically, the line I tested this change in EasyDynamics, which fixed the failing tests from easyscience/dynamics-lib#96 I didn't investigate why tests fail in this branch, but I expect that |
|
@rozyczko could you please merge this into your branch if it's ok? |
Fix is made to avoid hitting a race condition due to weakref.finalize() being called. It seems that .finalize temporarily deletes references of the object passed to it from dicts, therefore trying to get it from __type_dict or _store might randomly raise a KeyError.
This PR fixes failing tests from easyscience/dynamics-lib#96