Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

with old Python: instead of message informing about min required version, OCELOT drops long backtrace #234

Open
ZeugAusHH opened this issue Apr 24, 2024 · 12 comments

Comments

@ZeugAusHH
Copy link
Collaborator

Running OCELOT with very old Python versions gives a long backtrace instead of user friendly message "OCELOT requires Python 3.x or higher".

I tested this with v3.6.8 (which dates back to 2018) and is the default on MAXWELL cluster unless you choose to activate different version by loading modules. The test was done with the current status of OCELOT master branch, 18f64f2.

Probably a test for the minimum requirement should be added to https://github.com/ocelot-collab/ocelot/blob/master/ocelot/__init__.py .

demo script

[lechnerc@max-display008 20240424__ocelot_version]$ cat 20240424__ocelot_version.py
#!/usr/bin/env python3

# C. Lechner, EuXFEL, 2024-04-24

import sys
print('running '+sys.version)

# With old Python version this drops long backtrace.
# A message "OCELOT requires Python 3.x or higher" would be nicer...
import ocelot

from ocelot.utils.git_commit_id import get_git_commit_id
print('using OCELOT source code with git commit id: '+get_git_commit_id())

[lechnerc@max-display008 20240424__ocelot_version]$

With Python v3.9.16

[lechnerc@max-display008 20240424__ocelot_version]$ module list
Currently Loaded Modulefiles:
 1) maxwell   2) mamba/3.9
[lechnerc@max-display008 20240424__ocelot_version]$ ./20240424__ocelot_version.py
running 3.9.16 | packaged by conda-forge | (main, Feb  1 2023, 21:39:03)
[GCC 11.3.0]
[INFO    ] csr.py: module PYFFTW is not installed. Install it to speed up calculation.
initializing ocelot...
import: module PYFFTW is not installed. Install it to speed up calculation
using OCELOT source code with git commit id: 18f64f22baa9983cc5e2e259adb44ffa9e08fa6b

Now with very old Python version 3.6.8

[lechnerc@max-display008 20240424__ocelot_version]$ module purge
[lechnerc@max-display008 20240424__ocelot_version]$ module list
No Modulefiles Currently Loaded.
[lechnerc@max-display008 20240424__ocelot_version]$ ./20240424__ocelot_version.py
running 3.6.8 (default, Nov 14 2023, 16:29:52)
[GCC 4.8.5 20150623 (Red Hat 4.8.5-44)]
Traceback (most recent call last):
  File "./20240424__ocelot_version.py", line 10, in <module>
    import ocelot
  File "/[..]/ocelot/__init__.py", line 47, in <module>
    from ocelot.cpbd.magnetic_lattice import MagneticLattice, merger
  File "/[..]/ocelot/cpbd/magnetic_lattice.py", line 1, in <module>
    from ocelot.cpbd.elements.optic_element import OpticElement
  File "/[..]/ocelot/cpbd/elements/__init__.py", line 6, in <module>
    from ocelot.cpbd.elements.aperture import Aperture
  File "/[..]/ocelot/cpbd/elements/aperture.py", line 3, in <module>
    from ocelot.cpbd.elements.optic_element import OpticElement
  File "/[..]/ocelot/cpbd/elements/optic_element.py", line 2, in <module>
    from ocelot.cpbd.transformations.transfer_map import TransferMap
  File "/[..]/ocelot/cpbd/transformations/__init__.py", line 4, in <module>
    from ocelot.cpbd.transformations.cavity import CavityTM
  File "/[..]/ocelot/cpbd/transformations/cavity.py", line 3, in <module>
    from ocelot.cpbd.transformations.transfer_map import TransferMap, TMTypes
  File "/[..]/ocelot/cpbd/transformations/transfer_map.py", line 5, in <module>
    from ocelot.cpbd.transformations.transformation import Transformation, TMTypes
  File "/[..]/ocelot/cpbd/transformations/transformation.py", line 8, in <module>
    from ocelot.cpbd.beam import Particle, ParticleArray
  File "/[..]/ocelot/cpbd/beam.py", line 1670, in <module>
    class SliceParameters:
  File "/[..]/ocelot/cpbd/beam.py", line 1685, in SliceParameters
    "gamma_y": "gamma_y"}
TypeError: 'type' object is not subscriptable
[lechnerc@max-display008 20240424__ocelot_version]$
@st-walker
Copy link
Collaborator

st-walker commented Apr 24, 2024

If we don't support Python 3.6 (which I don't think we should, because it is ancient) then the solution is not to have a nice error when imported, but rather to not allow it to be installed in the first place. If somehow a user manages to install OCELOT on an ancient, unsupported version of Python (e.g. 2.7) and they have a nasty traceback, that will then be their problem.

OCELOT is installed using setuptools. We just need to add the right kwarg python_requires to the setup function:

https://packaging.python.org/en/latest/guides/dropping-older-python-versions/#specify-the-version-ranges-for-supported-python-distributions

Who wants to do it? Is it even really clear which Python versions we support exactly? @sergey-tomin is the expert here.

@ZeugAusHH
Copy link
Collaborator Author

I agree 100% that Python 3.6 shouldn't be supported.
Still, I do think that even if somebody clones from github, they should still get a nice message instead of the traceback. In the end this is just a simple if like all the other checks performed __init__.py.

I never implemented such version checking in setuptools, so I wouldn't implement it.

@st-walker
Copy link
Collaborator

I agree that it is a mistake that we have no way of preventing bad/old Pythons from being used. I prefer the python_requires as it adds to the already mostly declarative set of requirements in the setup.py rather than moving them into the code, which then needs to be remembered if we ever deprecate 3.7 (I think we should already have dumped anything older than 3.9 personally because numpy, matplotlib scipy all either require 3.9 or 3.10 at least, and we are not special).

To be honest we should probably bin the setup.py altogether and move to pyproject.toml.

I don't mind implementing something like this but I suppose it would be nice to understand what minor versions of Python we are actually going to support and which to bin.

@ZeugAusHH
Copy link
Collaborator Author

Concerning the minimum Python versions required by the different parts of OCELOT: Different parts may very well have different requirements. Probably @sergey-tomin and @sserkez know better.

But what about creating the "infrastructure" for it and just block out very old stuff such as the 3.6.x versions to begin with?

@st-walker
Copy link
Collaborator

In principle yes it's a nice idea but if the users need to pull from git and reinstall ocelot to benefit from this warning that their Python version is too old, then we might as well just cut to the chase and prevent them from installing in the first place.

@st-walker
Copy link
Collaborator

We need @sergey-tomin input to proceed with this, which Python versions we say we support and which we get rid of. I am not sure if we are even testing against all versions of Python we supposedly support.

@ZeugAusHH
Copy link
Collaborator Author

Assuming that it is not very much work, I would propose that we already build the framework for the Python version check. Users running 3.6.x could be stopped already now because it just doesn't work for 3.6.x (actually improving the situation for them because they would see a "nice" error message instead of that backtrace). And once we know what is the oldest officially supported version we adjust the version numbers...

By the way, I typically use OCELOT from a cloned directory that I keep up-to-date as needed. This enables me to quickly jump around between branches etc. So in my use case this version check would definitely be great.

@st-walker
Copy link
Collaborator

st-walker commented May 15, 2024

We have no hard definition of which Python OCLEOT even supports as far as I am aware, so first we would need that. Even if we did, I am opposed to this, I think it is not appropriate for OCELOT to start checking Python versions. I just checked numpy and they don't check for python version as far as I can tell. Llvmlite does the same, they do it at install time like I suggested originally.

By the way, I typically use OCELOT from a cloned directory that I keep up-to-date as needed

Me too. If we have minimum version in setup.py or pyproject.toml then we are fine. Beyond that, I don't even know how one would manage to mix python versions with ocelot, unless they are abusing PYTHONPATH, which is their problem and one should generally not be doing anyway.

@st-walker
Copy link
Collaborator

Hey @sergey-tomin and @ZeugAusHH i found this:

https://devguide.python.org/versions/#

versions 3.6 and 3.7 are now completely unsupported by Python. We shouldn't be using them nor supporting them, if not even Python itself uses/supports them. 3.8 will be dumped October this year. Let's get with the times and embrace the future together as one strong ocelot family.

@st-walker
Copy link
Collaborator

st-walker commented May 16, 2024

@sergey-tomin asked me to look into this so I did.

I tried installing ocelot starting with python3.6 and for each version check if ocelot imports and so on.

3.6

Doesn't work. Things this version hates about ocelot:

  • from __future__ import annotations
  • x = 3; f"{x=}"
  • x: dict[str, str]
  • Callable[int, int] as opposed to Callable[[int], int]

stopped editing ocelot at this point to find the next error, it doesn't work, move on.

3.7

  • x = 3; print(f"{x=})"

i deleted these lines and all tests passed

3.8

Doesn't even install on my m1 macbook. I suspect this is a problem i have with my python environment though rather than an ocelot problem, but i don't care at all about fixing this to find out more. the error:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/stuartwalker/repos/ocelot/ocelot/__init__.py", line 47, in <module>
    from ocelot.cpbd.magnetic_lattice import MagneticLattice, merger
  File "/Users/stuartwalker/repos/ocelot/ocelot/cpbd/magnetic_lattice.py", line 1, in <module>
    from ocelot.cpbd.elements.optic_element import OpticElement
  File "/Users/stuartwalker/repos/ocelot/ocelot/cpbd/elements/__init__.py", line 6, in <module>
    from ocelot.cpbd.elements.aperture import Aperture
  File "/Users/stuartwalker/repos/ocelot/ocelot/cpbd/elements/aperture.py", line 3, in <module>
    from ocelot.cpbd.elements.optic_element import OpticElement
  File "/Users/stuartwalker/repos/ocelot/ocelot/cpbd/elements/optic_element.py", line 2, in <module>
    from ocelot.cpbd.transformations.transfer_map import TransferMap
  File "/Users/stuartwalker/repos/ocelot/ocelot/cpbd/transformations/__init__.py", line 4, in <module>
    from ocelot.cpbd.transformations.cavity import CavityTM
  File "/Users/stuartwalker/repos/ocelot/ocelot/cpbd/transformations/cavity.py", line 3, in <module>
    from ocelot.cpbd.transformations.transfer_map import TransferMap, TMTypes
  File "/Users/stuartwalker/repos/ocelot/ocelot/cpbd/transformations/transfer_map.py", line 5, in <module>
    from ocelot.cpbd.transformations.transformation import Transformation, TMTypes
  File "/Users/stuartwalker/repos/ocelot/ocelot/cpbd/transformations/transformation.py", line 8, in <module>
    from ocelot.cpbd.beam import Particle, ParticleArray
  File "/Users/stuartwalker/repos/ocelot/ocelot/cpbd/beam.py", line 15, in <module>
    from ocelot.common.math_op import find_nearest_idx
  File "/Users/stuartwalker/repos/ocelot/ocelot/common/math_op.py", line 12, in <module>
    import numba as nb
  File "/Users/stuartwalker/Library/Python/3.8/lib/python/site-packages/numba/__init__.py", line 211, in <module>
    import numba.typed
  File "/Users/stuartwalker/Library/Python/3.8/lib/python/site-packages/numba/typed/__init__.py", line 1, in <module>
    from .typeddict import Dict
  File "/Users/stuartwalker/Library/Python/3.8/lib/python/site-packages/numba/typed/typeddict.py", line 23, in <module>
    def _make_dict(keyty, valty):
  File "/Users/stuartwalker/Library/Python/3.8/lib/python/site-packages/numba/core/decorators.py", line 260, in njit
    return jit(*args, **kws)
  File "/Users/stuartwalker/Library/Python/3.8/lib/python/site-packages/numba/core/decorators.py", line 183, in jit
    return wrapper(pyfunc)
  File "/Users/stuartwalker/Library/Python/3.8/lib/python/site-packages/numba/core/decorators.py", line 210, in wrapper
    disp = dispatcher(py_func=func, locals=locals,
  File "/Users/stuartwalker/Library/Python/3.8/lib/python/site-packages/numba/core/dispatcher.py", line 777, in __init__
    self.targetctx = self.targetdescr.target_context
  File "/Users/stuartwalker/Library/Python/3.8/lib/python/site-packages/numba/core/registry.py", line 47, in target_context
    return self._toplevel_target_context
  File "/Users/stuartwalker/Library/Python/3.8/lib/python/site-packages/numba/core/utils.py", line 277, in __get__
    res = instance.__dict__[self.name] = self.func(instance)
  File "/Users/stuartwalker/Library/Python/3.8/lib/python/site-packages/numba/core/registry.py", line 31, in _toplevel_target_context
    return cpu.CPUContext(self.typing_context)
  File "/Users/stuartwalker/Library/Python/3.8/lib/python/site-packages/numba/core/base.py", line 260, in __init__
    self.init()
  File "/Users/stuartwalker/Library/Python/3.8/lib/python/site-packages/numba/core/compiler_lock.py", line 35, in _acquire_compile_lock
    return func(*args, **kwargs)
  File "/Users/stuartwalker/Library/Python/3.8/lib/python/site-packages/numba/core/cpu.py", line 47, in init
    self._internal_codegen = codegen.JITCPUCodegen("numba.exec")
  File "/Users/stuartwalker/Library/Python/3.8/lib/python/site-packages/numba/core/codegen.py", line 1100, in __init__
    self._init(self._llvm_module)
  File "/Users/stuartwalker/Library/Python/3.8/lib/python/site-packages/numba/core/codegen.py", line 1105, in _init
    target = ll.Target.from_triple(ll.get_process_triple())
  File "/Users/stuartwalker/Library/Python/3.8/lib/python/site-packages/llvmlite/binding/targets.py", line 197, in from_triple
    raise RuntimeError(str(outerr))
RuntimeError: No available targets are compatible with triple "aarch64-apple-darwin23.2.0"

3.9

imports fine. didn't run tests. assume they pass since they passed in 3.7.

3.10+

Didn't check these. Python 3.12 works fine.

so there you go.

in any case i oppose adding code at import time to check for python 3.6, if you run python 3.6 this is a problem you have, e.g. also if you run ruby interpreter on ocelot code or if you try to compile it as c++.

@sergey-tomin
Copy link
Collaborator

Thank you @st-walker
So, what's your proposal? Do we support Python 3.8+ or...?
Regarding the traceback or nice message, I think highlighting the line with the supported Python version in README should be sufficient.

@st-walker
Copy link
Collaborator

bin 3.7 and 3.6. support 3.8 but in time we should bin 3.8 too. 3.9+ we carry on with.

Regarding the traceback or nice message, I think highlighting the line with the supported Python version in README should be sufficient.

yes it is not ocelot's job to help you if you use python 2 or something. it's not our problem. I propose we just update the README and close this issue.

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

No branches or pull requests

3 participants