Passing python version from "interpreter matrix" to mamba

[marcus-oscarsson]

Passing python version from interpreter matrix to mamba

[marcus-oscarsson]

Ok. Im not sure that this is the right way of doing things but now the tests at least runs with python 3.8.

If we think that this is a good, fix we should also do the same with the other actions, and possibly then add the other python versions to the matrix. What do you think ?

[marcus-oscarsson]

So I added python "3.9", "3.10", "3.11" so that we are certain that we are not introducing some incompatibility somewhere

[marcus-oscarsson]

Removed the default language so that we are using the python version of the virtual environment. This will make creation of the development environment and the management of the CI easier.

We will still run pre-commit with the lowest supported python version, currently python 3.8, in the CI

[fabcor-maxiv]

I am not convinced by this. Feels like there would be discrepancy between behaviour locally and on the CI, it might alienate people (I know I would be annoyed if tests pass locally but not on the CI).

[marcus-oscarsson]

I guess we finally don't need to update the UI action as the aim is not to test and lint python code. So this should be fine.

[fabcor-maxiv]

Thanks for working on this. This is something I have wanted to do for a long time but never got around to doing.

[marcus-oscarsson]

Ok, this looks good now :).

[marcus-oscarsson]

@rhfogh, @elmjag, @fabcor-maxiv @oldfielj-ansto :), what do you think ?

[elmjag]

I like this change!

[rhfogh]

This outside my expertise

[fabcor-maxiv]

I am not convinced by this. Feels like there would be discrepancy between behaviour locally and on the CI, it might alienate people (I know I would be annoyed if tests pass locally but not on the CI).

[marcus-oscarsson]

I am not convinced by this. Feels like there would be discrepancy between behaviour locally and on the CI, it might alienate people (I know I would be annoyed if tests pass locally but not on the CI).

Hm, so how should we manage pre-commit then ?

I thought that if there are some errors (and I guess it will happen in some occasions) one have to run pre-commit under python 3.8 sort of manually

Sort of related, are people testing against all python versions locally as well ?

[fabcor-maxiv]

I am not convinced by this. Feels like there would be discrepancy between behaviour locally and on the CI, it might alienate people (I know I would be annoyed if tests pass locally but not on the CI).

Hm, so how should we manage pre-commit then ?

That is tricky indeed. I will try to see how other projects solve this.

Maybe we should pin the Python version to our lowest supported version (3.8) in conda-environment.yml so that it is the one installed by default. People can still switch to a different version afterwards if they want to.

I thought that if there are some errors (and I guess it will happen in some occasions) one have to run pre-commit under python 3.8 sort of manually

I am not sure what you mean. If I understood correctly, with this change, what would happen is that pre-commit checks would be fine locally but would fail on GitHub. From there, I am not sure people would immediately think they need to rewrite code for Python 3.8. That is not obvious.

Sort of related, are people testing against all python versions locally as well ?

No, I do not, and I do not think my colleagues do it either. Typically in some projects I have seen, tox or nox is used and that makes this very easy to do locally. I am not sure we should do this for MXCuBE, though.

[marcus-oscarsson]

With this change pre-commit would run with the python version installed in the environment, just like the tests will. So in that sense its consistent.

If we pin python to 3.8 in conda-environment.yml most of us would need to change the python version and then in anyways also have one environment for python 3.8 to run pre-commit. I'm not sure it would be much of a difference for most of us ?

[marcus-oscarsson]

What I suggest is that, if you agree, we merge this for now. You can follow up with neater solution for how to handle the pre-commit when you find one ?

[fabcor-maxiv]

What I suggest is that, if you agree, we merge this for now. You can follow up with neater solution for how to handle the pre-commit when you find one ?

Yes, I am fine with merging this, all in all it is an improvement because it catches things that used to slip through. The discrepancy issue I mentioned would probably be rare, and at least it would be caught, not slip through.