search

pydata / patsy

Describing statistical models in Python using symbolic formulas
Other
955 stars 104 forks source link

remove Python2 support #209

Closed a-detiste closed 2 weeks ago

a-detiste commented 5 months ago

patsy is still needed in Debian because it's a dependency of something else

bashtage commented 5 months ago

patsy is more-or-less in permanent maintenace mode. I'm not sure that this patch can be accepted given the breadth of the changes needed. Why is this needed?

a-detiste commented 5 months ago

Hi, it's mostly about untangling the six dependencies. I've sent tens (hundreds ?) so PR, so I'm used to even if it looks a bit scary.

a-detiste commented 5 months ago

Removing declared six dependencies help to find undeclared six dependencies somewhere else, that's the most interesting point. So many people think that six is from the standard library and should always be there.

I think that this lib will stay forever, but limiting the risk coverage for distributions would be a nice move image

bashtage commented 5 months ago

I don't think that removing six is a strong enough reason to accept these changes, but others who have helped continue this library should have their say.

pgajdos commented 2 weeks ago

On the contrary, I do not see strong enough reason to keep python2 compatibility. Could you please reconsider?

pgajdos commented 2 weeks ago

I don't think that removing six is a strong enough reason to accept these changes, but others who have helped continue this library should have their say.

CC @matthewwardrop, probably?

pgajdos commented 2 weeks ago

For openSUSE Tumbleweed, there seems to be only one consumer: python-statsmodels, which is python3 only.

pgajdos commented 2 weeks ago

I can confirm that the patch works well for me. https://build.opensuse.org/package/show/home:pgajdos:python/python-patsy Thanks @a-detiste

bashtage commented 2 weeks ago

For openSUSE Tumbleweed, there seems to be only one consumer: python-statsmodels, which is python3 only.

I am not worried about distributions that put together packages since these are usually have pretty tight bounds. The concern is about PyPI.

I still don't see any real upside for this patch given the status of patsy.

matthewwardrop commented 2 weeks ago

Hi @a-detiste !

As indicated by others above, patsy is pretty deep maintenance mode; but I'm not averse to making necessary changes to keep things working for people who need this package. With that in mind, why is removing six important? I imagine the six library will be around for a long time to come.

It does remind me that I need to spend the time to help port statsmodels and other libraries to formulaic... but my time these days is pretty tightly constrained.

a-detiste commented 2 weeks ago

Hi, I spent best of last year tracking all these old libraries. So many were backports of parts of Python3 to Python2. Even after the removal of Python2 runtime itself, the distributions are left with a lot of scar tissue from the not-so-good Py2>3 migration to manage. Any of those libraries may/will start to fail with some newer python version.

https://wiki.debian.org/Python/Dead%20Batteries : a lot have been removed completely already

six is one of those old libraries, removing it is a very slow job. Sometimes it's the whole library using six that gets removed. Here we are lucky: you are here to read us and tag a release.

For abandoned packages each distribution need to come up with a patch.

So in this case tagging a release with this change would enable an economy of scale between OpenSuse & Debian work. The other distro would benefit it only. I know some other cleanser busy with Gentoo, after a while we see each others PR.

Right now six is still to big to fail so will be fixed whatever it cost. An counter-example is Nose testing framework that has been dead upstream for 10 years: we strongly consided disabling the test suite of the last 10-something packages altogether.

A lot of things uses statsmodel so statsmodel is there to stay

bashtage commented 2 weeks ago

A lot of things uses statsmodel so statsmodel is there to stay

statsmodels is going to move to formuliac (both for long-term support and new features), although it will be a while.

matthewwardrop commented 2 weeks ago

I took a look through the PR in more detail, and I'm fine with merging this in. I don't see anything concerning. I still don't think it is necessary, per-say, but there's no reason to carry around Python 2 support at this point, and the changes here are all benign. I'll release it as 0.7, along with a fix for numpy 2.0.0 compatibility.

bashtage commented 2 weeks ago

Is it missing a python_requires directive in setup now?

bashtage commented 2 weeks ago

Should be 3.6+ since this is all that it is tested against.

a-detiste commented 2 weeks ago

Follow up: https://github.com/pydata/patsy/pull/213

pgajdos commented 2 weeks ago

Thank you guys

matthewwardrop commented 2 weeks ago

Thanks all! And yes, there's a few metadata pieces needing changing. Thanks for putting together the other PR, which I'll review shortly.