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

Drop Python 3.7 support and update tests #1316

Merged
merged 16 commits into from
Oct 25, 2024

Conversation

Darkbeast-glitch
Copy link
Contributor

Thank you for taking your time to contribute to Ersilia, just a few checks before we proceed

  • Have you followed the guidelines in our Contribution Guide
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Description

Updated the pyproject.toml file to drop all dependencies support 3.7 , removed tests concerned with python version 3.7
Changes to be made

Replace this line with how do you intend to go about getting this done...

Status

Updated the pyroject.toml file and changed all python version from 3.7 to 3.8
To do

If this is a work in progress, Replace this line with your next steps

Is this pull request related to any open issue? If yes, replace issueID below with the issue ID

Related to #1226

@Darkbeast-glitch
Copy link
Contributor Author

@DhanshreeA please can you check this out please

@@ -10,7 +10,6 @@ repository = "https://github.com/ersilia-os/ersilia"
documentation = "https://ersilia.io/model-hub"
keywords= ["drug-discovery", "machine-learning", "ersilia", "open-science", "global-health", "model-hub", "infectious-diseases"]
classifiers=[
"Programming Language :: Python :: 3.7",
"Programming Language :: Python :: 3.8",
"Programming Language :: Python :: 3.9",
"Programming Language :: Python :: 3.10",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about added support for python 3.11 here since you've already added it in your code above

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so you mean I should add python version 3.11 right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While you are at it, please mention 3.12 as well, thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure please on it

@@ -11,7 +11,7 @@ jobs:
install-ersilia:
strategy:
matrix:
python-version: ["3.7", "3.8", "3.9", "3.10", "3.11"]
python-version: ["3.8", "3.9", "3.10", "3.11"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can also add 3.12 to this list since we support 3.12 as well.

@@ -32,7 +32,7 @@ jobs:
test-docker:
strategy:
matrix:
python-version: ["3.7", "3.8", "3.9", "3.10", "3.11"]
python-version: ["3.8", "3.9", "3.10", "3.11"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, we can include 3.12 as well.

@@ -10,7 +10,6 @@ repository = "https://github.com/ersilia-os/ersilia"
documentation = "https://ersilia.io/model-hub"
keywords= ["drug-discovery", "machine-learning", "ersilia", "open-science", "global-health", "model-hub", "infectious-diseases"]
classifiers=[
"Programming Language :: Python :: 3.7",
"Programming Language :: Python :: 3.8",
"Programming Language :: Python :: 3.9",
"Programming Language :: Python :: 3.10",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While you are at it, please mention 3.12 as well, thank you!

pyproject.toml Outdated
@@ -49,7 +47,7 @@ numpy = "<=1.26.4"
isaura = {version="0.1", optional=true}
pytest = {version = "^7.4.0", optional = true}
fuzzywuzzy = {version = "^0.18.0", optional = true}
sphinx = {version = ">=5.3.0", optional = true} # For compatibility with python 3.7.x
sphinx = {version = ">=5.3.0", optional = true}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we actually check which is the minimum version we need for 3.8?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok sure

@Darkbeast-glitch
Copy link
Contributor Author

@DhanshreeA please I made the changes and also worked on your request, kindly verify and let me know if there is anything i need to do , thank you

@Darkbeast-glitch
Copy link
Contributor Author

Darkbeast-glitch commented Oct 14, 2024

@DhanshreeA Hello ma, please I am seeing some failures, working to resolve that please.

@DhanshreeA
Copy link
Member

@Darkbeast-glitch could you please resolve the merge conflicts?

@Darkbeast-glitch
Copy link
Contributor Author

@Darkbeast-glitch could you please resolve the merge conflicts?

sure please on it

@Darkbeast-glitch
Copy link
Contributor Author

@DhanshreeA please I am getting some errors , I think ersilia is failing to install on the python version 12

@Ajoke23
Copy link
Contributor

Ajoke23 commented Oct 17, 2024

@Darkbeast-glitch could you post the error you are getting?

@Darkbeast-glitch
Copy link
Contributor Author

@Darkbeast-glitch could you post the error you are getting?

It's coming from the checks, I think 2 are failing, and after checking it's due to the introduction of the python version 3.12

@Ajoke23
Copy link
Contributor

Ajoke23 commented Oct 18, 2024

But ersilia support python version 3.12. Did you try to remove 3.12 from the list and run the test, was it successful?

@Darkbeast-glitch
Copy link
Contributor Author

Darkbeast-glitch commented Oct 21, 2024

@DhanshreeA please check this if you get time please. Thank you

@Darkbeast-glitch
Copy link
Contributor Author

@DhanshreeA should I drop the python version 3.12, cause ersilia keeps failing to install on that python version , I have Traced back the issue and I added the setup tools buh it still give me that error

@DhanshreeA
Copy link
Member

DhanshreeA commented Oct 25, 2024

Hi @Darkbeast-glitch I've pushed a fix in the culprit file where we were getting the error from. This should make even Python 3.12 work. For reference, the module pkg_resources has been deprecated entirely in favor of importlib in Python 3.12. I have updated the code to use importlib as a drop-in replacement for pkg_resources. My apologies this has taken so long.

@DhanshreeA DhanshreeA merged commit c1a974d into ersilia-os:master Oct 25, 2024
18 checks passed
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.

4 participants