madanalysis5 icon indicating copy to clipboard operation
madanalysis5 copied to clipboard

Change in importlib

Open jackaraz opened this issue 2 years ago • 8 comments

System Settings

for python version > 3.9

Describe the bug

Importlib will crash. The find_spec function moved from the util module see this link.

To Reproduce

run ma5

Expected behaviour

No response

Log files

No response

Additional information

No response

jackaraz avatar Jan 18 '24 12:01 jackaraz

Hello Jack,

I was just going through madanalysis installation and was going to report this but I found that you already took note of the same issue. it works if lines change like below

from importlib import util

...

# Checking that the 'six' package is present
if not util.find_spec("six"):

sihyunjeon avatar Feb 04 '24 20:02 sihyunjeon

Hi @sihyunjeon, thanks for the comment! I just need to make sure that it will work in multiple versions of Python at the same time. Probably will need to implement some kind of error handling, will see, thanks!

jackaraz avatar Feb 05 '24 03:02 jackaraz

Dev Note: pkg_resources has a get_distribution function, which can be used to check existing packages. If a package is not found, it raises the pkg_resources.DistributionNotFound error. This is more efficient than importing packages. I believe pkg_resources is universal for all Python versions (needs to be checked)

jackaraz avatar Feb 06 '24 03:02 jackaraz

hi @jackaraz Do you plan to re-release v.1.10.12 or should i wait for v1.10.13 to adapt to this change?

sihyunjeon avatar Feb 11 '24 18:02 sihyunjeon

Hi @sihyunjeon, we will release a new version, but we are all busy atm, so it might take some time along with other planned releases.

jackaraz avatar Feb 11 '24 19:02 jackaraz

There is already a check of python supported versions at the start of bin/ma5

import importlib
import os
import sys

# Checking if the correct release of Python is installed
if sys.version_info[0] != 3 or sys.version_info[1] <= 6:
    sys.exit(
        "Python release "
        + sys.version
        + " is detected.\n"
        + "MadAnalysis 5 works only with Python version 3.6 or more recent version.\n"
        + "Please upgrade your Python installation."
    )

by adding a minor version check the issue can be solved as purposed here

if sys.version_info[1] >= 9:
  import importlib.util

and the code would work as intended.

Actually that would work with any supported version because explicitly importing a submodule exposed in the __init__.py of importlib module is not an overhead, it just looks a bit weird, but a comment explaining the hack should be enough.

In the end it can look like

import importlib.util  # Python >= 3.9 compatibility issue (https://github.com/MadAnalysis/madanalysis5/issues/237) 
import importlib
import os
import sys

# Checking if the correct release of Python is installed
if sys.version_info[0] != 3 or sys.version_info[1] <= 6:
    sys.exit(
        "Python release "
        + sys.version
        + " is detected.\n"
        + "MadAnalysis 5 works only with Python version 3.6 or more recent version.\n"
        + "Please upgrade your Python installation."
    )

I'm happy to makea a PR with the fix.

davo9819 avatar Jul 20 '24 23:07 davo9819

Actually I just tested python 3.6, python 3,7 and python3.8 and all failed with AttributeError: module 'importlib' has no attribute 'util'.

The problem seems to be that some libraries bring importlib.util to scope (like REPL) silently and user code works, but with a clean venv this is not the case.

So an actual fix would be explicitly import importlib.util with every version of python.

davo9819 avatar Jul 20 '24 23:07 davo9819

Thanks, @davo9819; I'll take a look when I have some time!

jackaraz avatar Jul 21 '24 18:07 jackaraz