-
Notifications
You must be signed in to change notification settings - Fork 85
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
Use ComparisonMode constants instead of magic numbers #1229
Conversation
Note: this PR will conflict with #1165; whichever one gets merged second (probably this one) will need to deal with the merge conflicts. |
Hmm. I somehow lost some of the changes when resolving conflicts. Will fix. |
Ah no; I see - there were new cases introduced in #1208. |
@@ -417,12 +422,12 @@ class Potato(HasTraits): | |||
|
|||
class PotatoBag(HasTraits): | |||
|
|||
potatos = List(Instance(Potato), comparison_mode=1) | |||
potatos = List(Instance(Potato), comparison_mode=ComparisonMode.identity) |
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 really want to change this to potatoes
, but that's out of scope for this PR.
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.
Oops. 😂
Race conditions from a number of PRs. Yes I agree the many |
@kitchoi Okay to merge this and look at removing redundant |
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.
Yep! LGTM
Minor cleanup PR: replace uses of magic numbers for comparison modes with the corresponding
ComparisonMode
elements. (The numbers should eventually become just an implementation detail; we want to encourage all traits users to use theComparisonMode
constants instead.)