search

SeisComP / seedlink

Seedlink server to be built within SeisComP
Other
13 stars 17 forks source link

fixed imp compatibility issues for python 3.12 and higher #15

Closed Donavin97 closed 5 months ago

Donavin97 commented 5 months ago

Good day everyone. I am making another PR to fix an issue where the 'imp' module was removed from python 3.12. This fix uses the 'importlib' module to handle the creation of dynamic modules within the seedlink namespace. This should work for all python 3.x versions, although it has only been tested on python3.12. thanks.

gempa-jabe commented 5 months ago

According to the documentation importlib has been introduced with Python 3.1, see https://docs.python.org/3/library/importlib.html. I think that it won't break any installation as we are requiring Python 3 and I don't think that any distribution ships with Python 3.0. We will check. Thank you for the fix.

gempa-stephan commented 5 months ago

The minimum required Python version is 3.6.8.

gempa-jabe commented 5 months ago

Now I am realising that we are in the seedlink repo. In seiscomp-control.py we have been using importlib for quite some time. Wouldn't the change boild down to

-- mod = imp.new_module(modname)
++ mod = importlib.import_module(modname)

?

Donavin97 commented 5 months ago

Hi there Jan. I pushed the change that you suggested. Thanks everyone. Now SeisComP is working perfectly in python 3.12.

jsaul commented 5 months ago

Thanks a lot, Donavin!

gempa-jabe commented 5 months ago

Let's merge then. Thanks @Donavin97.

gempa-jabe commented 5 months ago

@Donavin97, I have tested this again and actually your initial commit was correct. The revised version is just wrong as the module does not exist on the filesystem and your initial approach was absolutely correct. I will revert the revised commit to make it work again.

Donavin97 commented 5 months ago

Hi Jan, Thanks for your feedback and for taking the time to test my contribution. Everything is good with me. Go ahead and revert back to the initial commit. I will keep testing on my end as well. Regards: Donavin On Tue, May 07, 2024 at 01:55:22AM -0700, Jan Becker wrote:

@Donavin97, I have tested this again and actually your initial commit was correct. The revised version is just wrong as the modules does not exist on the filesystem and your initial approach was absolutely correct. I will revert the revised commit to make it work again.

-- Reply to this email directly or view it on GitHub: https://github.com/SeisComP/seedlink/pull/15#issuecomment-2097789243 You are receiving this because you were mentioned.

Message ID: @.***>