Skip to content
This repository was archived by the owner on Dec 6, 2023. It is now read-only.

Run tests with actual Python versions #161

Merged
merged 1 commit into from
May 5, 2021
Merged

Run tests with actual Python versions #161

merged 1 commit into from
May 5, 2021

Conversation

StrikerRUS
Copy link
Contributor

@StrikerRUS StrikerRUS commented May 4, 2021

https://devguide.python.org/#status-of-python-branches

All other versions have reached their EOL.

@StrikerRUS StrikerRUS marked this pull request as ready for review May 4, 2021 23:26
@mblondel
Copy link
Member

mblondel commented May 5, 2021

Looks OK on my side. @vene may want to take a look too.

@vene
Copy link
Contributor

vene commented May 5, 2021

this implies dropping support for 2.x right?

are the CI failures solved in another PR?

@StrikerRUS
Copy link
Contributor Author

this implies dropping support for 2.x right?

Right now just stop testing Python 2 and old Python 3 versions.

In further PRs I can help to actually drop Python 2 support and dependency from six (these will be breaking changes).

are the CI failures solved in another PR?

Please refer to #163 (comment). So yes, I'm trying to fix CI failures iteratively with series of self-contained focused on one problem PRs.

@mblondel mblondel merged commit 1329a57 into scikit-learn-contrib:master May 5, 2021
@vene
Copy link
Contributor

vene commented May 5, 2021

thanks for clarifying!

i think it's fair to consider untested = unsupported officially, so maybe we should update the readme.

this pr cleans up the CI scripts very nicely, thank you!

@StrikerRUS
Copy link
Contributor Author

@vene

i think it's fair to consider untested = unsupported officially,

Let me kindly disagree with you, but I totally understand your vision though. I'd better to distinguish untested and unsupported like the following.

untested

"Hey, we don't know whether this works on Python 2 or not, because we don't run tests against it. But you can try and maybe have a luck to get it work!"

unsupported

Python 2 is not supported because (for example) we use f-strings in our codebase which are supported only in 3.6+ version.

@vene
Copy link
Contributor

vene commented May 7, 2021

I see your point, but I think what you are saying does not mean python2 is supported, it just means "code may run on python 2.

Support means a bit more than just "belief it runs without throwing exceptions". Support means also that we can answer bug reports and that if there is some new feature PR in a year or so (e.g. something containing f-strings), we keep py2 support. And the only way to do these things reliably is by running tests on py2.

Leaving open a gap and saying "this might work but you're completely on your own" is not a great idea IMO. Libraries have agonized over this for a long time before py2 was sunset. Now that it has, I think it's ok and better to officially drop support.

@StrikerRUS
Copy link
Contributor Author

I see your point, but I think what you are saying does not mean python2 is supported, it just means "code may run on python 2.

Exactly!

Now that it has, I think it's ok and better to officially drop support.

Great!

In further PRs I can help to actually drop Python 2 support and dependency from six (these will be breaking changes).
#161 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants