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

remove "import imp/import importlib hybridation" #381

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

a-detiste
Copy link
Contributor

@a-detiste a-detiste commented Sep 15, 2024

this is an other deprecated & removed module

@joeraz
Copy link
Collaborator

joeraz commented Sep 15, 2024

So these changes break Python 2 compatibility (which I'm okay with, but it will need to be documented). Can you confirm what the new minimum version of Python to run this updated code would be for said documentation?

@a-detiste
Copy link
Contributor Author

Hi,

I included this in the previous PR, it can be documented at other places too:

'classifiers': [
        'Programming Language :: Python :: 3',
    ],

@a-detiste
Copy link
Contributor Author

Importlib was backported to python2.7 ... this is very old compatibility code

@joeraz
Copy link
Collaborator

joeraz commented Sep 15, 2024

Yes, but I'm thinking of updating the documentation in files like https://github.com/shlomif/PySolFC/blob/master/README.md or https://github.com/shlomif/PySolFC/blob/master/CONTRIBUTING.md where the Python version requirements are stated for users and contributors.

So would 3.0 be the minimum Python version then?

@joeraz
Copy link
Collaborator

joeraz commented Sep 18, 2024

Tested and it works. Merging.

Also tested with Python 3.7. No issues there either - I'll just need to confirm whether the minimum version would be 3.7, 3.0, or something in between.

@joeraz joeraz merged commit 83baa54 into shlomif:master Sep 18, 2024
2 checks passed
@a-detiste
Copy link
Contributor Author

Hi. 3.7 as a minimal supported version today is a common/safe choice.

3.0 got very little real world use, even back then (none of the library were ported) and feels weird as an explitly supported version.

@joeraz
Copy link
Collaborator

joeraz commented Sep 19, 2024

Noted. I'll do that then.

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

Successfully merging this pull request may close these issues.

2 participants