-
-
Notifications
You must be signed in to change notification settings - Fork 200
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
Take some of the magic out of traits by insisting traits be instances, not classes #51
Conversation
This will require quite a few changes to other repos, because things like |
@@ -64,7 +64,8 @@ | |||
#----------------------------------------------------------------------------- | |||
# Basic classes | |||
#----------------------------------------------------------------------------- | |||
|
|||
import warnings | |||
warnings.filterwarnings('error') |
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.
This line, changing warnings to errors, should be removed for the final version of this PR. This just makes it really easy to see where things break.
Jason reviews his own pull requests :) I am 👍 on this change. This deprecates half of the code of the metaclass. |
Hey, I write them with my right brain, I review them with my left! |
Yes, I think the deprecation should happen in 4.1, removal in 5.0 |
Sounds good. |
I am +1 on this change. On Thu, Jul 16, 2015 at 3:11 PM, Min RK notifications@github.com wrote:
Brian E. Granger |
361e7f6
to
e9959b2
Compare
…, not classes e.g., Int(), not Int.
I think this (and the related commits in different repos noted above) are ready for review now. |
ping on merging this. Because both this and the metadata PR change the same lines when upgrading our code, I'd like to base the metadata pr on this one, and that's easier if this is merged into master. |
Take some of the magic out of traits by insisting traits be instances, not classes
cool! |
As Sylvain proposed in #48,