search

scikit-learn-contrib / lightning

Large-scale linear classification, regression and ranking in Python
https://contrib.scikit-learn.org/lightning/
1.73k stars 214 forks source link

Run tests with actual Python versions #161

Closed StrikerRUS closed 3 years ago

StrikerRUS commented 3 years ago

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

All other versions have reached their EOL.

mblondel commented 3 years ago

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

vene commented 3 years ago

this implies dropping support for 2.x right?

are the CI failures solved in another PR?

StrikerRUS commented 3 years ago

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 https://github.com/scikit-learn-contrib/lightning/pull/163#issuecomment-832820494. So yes, I'm trying to fix CI failures iteratively with series of self-contained focused on one problem PRs.

vene commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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). https://github.com/scikit-learn-contrib/lightning/pull/161#issuecomment-832823506