-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Added type hints to the Column class #3511
Conversation
morozov
commented
Apr 7, 2019
Q | A |
---|---|
Type | improvement |
BC Break | yes |
b848bc5
to
f7c6461
Compare
f7c6461
to
22aa64e
Compare
Can we (should we?) separate the BC breaks due to adding type hints and BC breaks from us changing default behavior? IMO, while changing the defaults may be a good idea long term, I am not sure if making the change along side adding type hints is good. May be better to have 1 PR to add the type hints while maintaining behavior and then another PR to change the defaults. This way the defaults changing can be discussed in isolation from all the other changes and the type hints can probably be merged faster. If I am missing something, feel free to correct me. Just starting to get up to speed with the great work going on here. |
@jwage I get the intent but if we want to take this route, the changes have to be implemented in the reverse order:
The existing defaults only make sense with non-enforced argument types (hence the I like the idea of the split in general. Especially, given that there are still some leftovers of the default value of |
22aa64e
to
57fe3c6
Compare
Added type hints to the Column class
Added type hints to the Column class
Added type hints to the Column class
Added type hints to the Column class
Added type hints to the Column class
Added type hints to the Column class
Added type hints to the Column class
Added type hints to the Column class
Added type hints to the Column class
Added type hints to the Column class
Added type hints to the Column class