-
Notifications
You must be signed in to change notification settings - Fork 36
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 usage of _convert_to_dict to avoid modifying nesting SyncedDict objects when calling items or values #269
Conversation
Codecov Report
@@ Coverage Diff @@
## master #269 +/- ##
==========================================
- Coverage 65.42% 65.07% -0.35%
==========================================
Files 42 40 -2
Lines 5697 5629 -68
==========================================
- Hits 3727 3663 -64
+ Misses 1970 1966 -4
Continue to review full report at Codecov.
|
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.
👍
@vyasr Can you update the name of this PR to be a bit more descriptive? |
Yes, sorry it was a single commit so it just autopopulated the title and I didn't change it. |
_convert_to_dict
to avoid modifying nesting SyncedDict objects when calling items
or values
_convert_to_dict
to avoid modifying nesting SyncedDict objects when calling items
or values
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.
Looks good!
Description
A number of methods were calling
_convert_to_dict
onself._data
rather than calling them onself
. As a result, the member of the underlying dictionary was modified. Actually, we should be able to remove thedict
branch of_convert_to_dict
entirely, but that requires resolving #196 first (because that's the only way that branch is accessed). Further changes are probably worth postponing for when we address #249.Motivation and Context
Resolves #234.
Types of Changes
1The change breaks (or has the potential to break) existing functionality.
Checklist:
If necessary:
Example for a changelog entry:
Fix issue with launching rockets to the moon (#101, #212).