Skip to content

MNT redundant from __future__ import #13079

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Feb 3, 2019
Merged

MNT redundant from __future__ import #13079

merged 2 commits into from
Feb 3, 2019

Conversation

qinhanmin2014
Copy link
Member

Closes #12779, missing in #12791
Remove all from future imports (apart from sklearn/externals/)

@adrinjalali
Copy link
Member

it wasn't missing from #12791. Removing this __future__ import breaks the tests (at least last time we checked it did)!

@qinhanmin2014
Copy link
Member Author

it wasn't missing from #12791. Removing this future import breaks the tests (at least last time we checked it did)!

You're right. Do you know the reason? I think we should fix it.

@adrinjalali
Copy link
Member

haven't had time to investigate, but I agree that it needs to be fixed. I kinda didn't touch it cause I was afraid it'll result in some odd backward compatibility issues.

@qinhanmin2014
Copy link
Member Author

OK, I found #12796 and #12873

@qinhanmin2014
Copy link
Member Author

@adrinjalali @rth let's first merge this one to remove all the future imports?

@rth
Copy link
Member

rth commented Feb 3, 2019

let's first merge this one to remove all the future imports?

This PR removes one such import. I'm fine merging it, should it really auto-close #12779 though ? I'm fine with that issue being closed after #12791 and be done with it -- just feels strange to close it with a single change to cython files...

@qinhanmin2014
Copy link
Member Author

I'm fine with that issue being closed after #12791 and be done with it

@rth #12791 is already merged. there's no future imports after this PR (apart from sklearn/externals/)

@rth
Copy link
Member

rth commented Feb 3, 2019

You are right.

@qinhanmin2014 qinhanmin2014 merged commit abd91d4 into scikit-learn:master Feb 3, 2019
@qinhanmin2014 qinhanmin2014 deleted the redundant-import branch February 5, 2019 00:31
thomasjpfan pushed a commit to thomasjpfan/scikit-learn that referenced this pull request Feb 6, 2019
@qinhanmin2014 @thomasjpfan
MNT redundant from __future__ import (scikit-learn#13079)
82b7cab
thomasjpfan pushed a commit to thomasjpfan/scikit-learn that referenced this pull request Feb 7, 2019
@qinhanmin2014 @thomasjpfan
MNT redundant from __future__ import (scikit-learn#13079)
92e93d7
xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019
@qinhanmin2014
MNT redundant from __future__ import (scikit-learn#13079)
0dfad71
xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019
Revert "MNT redundant from __future__ import (scikit-learn#13079)"
ce116d1 
This reverts commit 0dfad71.
xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019
Revert "MNT redundant from __future__ import (scikit-learn#13079)"
b8dfc3f 
This reverts commit 0dfad71.
koenvandevelde pushed a commit to koenvandevelde/scikit-learn that referenced this pull request Jul 12, 2019
@qinhanmin2014 @koenvandevelde
MNT redundant from __future__ import (scikit-learn#13079)
879b919
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rth rth rth approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@qinhanmin2014 @adrinjalali @rth