Python Major Versions that can be used

[psteinb]

as lie_learn currently cannot be installed with python 3.11 and above

[psteinb]

see also AMLab-Amsterdam/lie_learn#26

[psteinb]

Note, I checked the installation of the package with py 3.7 and 3.11. The former works as expected, the latter bails out as python 3.11 is not supported currently.

According to the docs, this change once pushed to PyPI should also affect people running pip install escnn from python 3.11.

[kalekundert]

I just noticed this PR, and I haven't looked at it too closely, but I don't think it makes sense for escnn to require python<3.11 (assuming that escnn works with python>=3.11 as long as lie_learn is installed). It's not like it's impossible to install lie_learn for python>=3.11, it just requires an arcane set of pip commands. And installing lie_learn for python 3.9-3.10 with an up-to-date version of pip also doesn't work out-of-the-box, so it's not like the problem with lie_learn is confined to python>=3.11.

If anything, it might be possible for escnn to specify its dependencies in such a way that installing lie_learn just works. Specifically, this would mean (i) requiring wheel, (ii) requiring cython<3.0, and (iii) requiring the github version of lie_learn rather than the PyPI version. I'm not totally sure that this would work, though, because the first two have to be installed before lie_learn, and I don't think setuptools makes any guarantees about the order in which it installs dependencies. I don't think this would be worth the trouble, though, because the whole problem should go away once AMLab-Amsterdam/lie_learn#26 is merged. It's probably about time to ping the maintainer about that, though.

[psteinb]

Good discussion! While being a bit blunt, I suggest this PR to prevent headaches for people lacking the capabilities to deal with the deficiencies of lie_learn. That is the core motivation.

[kalekundert]

I agree with that motivation, but I worry that this approach will just shift the headaches to people who want to use up-to-date python. Especially since, if the goal is to avoid lie_learn headaches, then the requirement would have to be python<=3.8 rather than 3.10, since that's the last version that can install lie_learn with up-to-date pip.

I'd prefer to either (i) do nothing and wait for lie_learn to be fixed or (ii) have setup.py print out an error message explaining the problem with lie_learn and the commands needed to install it. In fact, now that I think about it, maybe the best thing would be to change the lie_learn dependency to point to my fork, e.g.:

# setup.py
install_requires = [
    ...
    "lie_learn @ git+https://github.com/kalekundert/lie_learn@8f89f11",
    ...
]

Not totally sure that that's the right syntax, but in principle that would immediately solve the problem, and it'd be easy to switch back to the real lie_learn whenever it's fixed.

[psteinb]

I just checked, @kalekundert suggestion works like a charm for a local pip install . As I don't want to take the credit, I suggest @kalekundert sends an autonomous PR, if @Gabri95 is fine with relying on the lie_learn fork by @kalekundert at the moment.