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

🐛 Bug: Ersilia Test on PY3.12 #1494

Closed
GemmaTuron opened this issue Jan 8, 2025 · 8 comments · Fixed by #1500
Closed

🐛 Bug: Ersilia Test on PY3.12 #1494

GemmaTuron opened this issue Jan 8, 2025 · 8 comments · Fixed by #1500
Assignees
Labels
bug Something isn't working

Comments

@GemmaTuron
Copy link
Member

Describe the bug.

I am trying to use the test command at Ersilia. I have tried to install it with
pip install -e .[test]
In an environment that has Python 3.12, as in principle Py3.12 is supported by Ersilia. I have run into the following errors:

  1. The rdkit-pypi I think is downgraded, I get the following error that disappears if I change lines 63 and 73 of the pyproject.toml file:
INFO: pip is looking at multiple versions of ersilia to determine which version is compatible with other requirements. This could take a while.
ERROR: Could not find a version that satisfies the requirement rdkit-pypi (from ersilia) (from versions: none)
ERROR: No matching distribution found for rdkit-pypi
  1. Once this is bypassed I get stuck at this issue (attached .txt).
    ersiliatest_py312.txt

I believe this is due to Python 3.12 because in an environment with 3.11 it works. Also, The Ersilia install without [test] seems to work in Py3.12. Tagging both @Abellegese as the developer of the testing and @DhanshreeA

Describe the steps to reproduce the behavior

No response

Operating environment

Ubuntu 24.04 LTS

@GemmaTuron GemmaTuron added the bug Something isn't working label Jan 8, 2025
@DhanshreeA
Copy link
Member

Wait, this is clearly an error. rdkit is never supposed to be explicitly installed in ersilia, and therefore, not to be explicitly specified in the pyproject.toml at all. I see this was added as an optional test extra within ersilia, ie not to be installed with the standard ersilia installation. But in either case, the test should not require it either since ersilia takes care of installing rdkit whenever it is doing anything with compound inputs. In fact, even within that part of the code, we switched to using the more updated versions of rdkit.

@DhanshreeA
Copy link
Member

I think first step is to make sure that the test extra should not need rdkit, ie the test should go through the flow of installing rdkit the way ersilia normally does when dealing with compound inputs.
Secondly, if for whatever reason, rdkit is explicitly required, then we need to use rdkit>=2023 onward and definitely not rdkit-pypi.

@GemmaTuron
Copy link
Member Author

so the rdkit-pypi was added in this commit which was authored by you @DhanshreeA and @Abellegese if I am not wrong: 4f7edf4#diff-50c86b7ed8ac2cf95bd48334961bf0530cdc77b5a56f852c5c61b89d735fd711

@Abellegese
Copy link
Contributor

Abellegese commented Jan 9, 2025

Yes @GemmaTuron I did that because of the new pytest functionality. We have partial mocking on the test_run.py, meaning the run is taking place without serving (we test CLI components, and few functional level testing of run command). Apparently rdkit was installed during serving:

def setup(self):
    RdkitRequirement()
    ChemblWebResourceClientRequirement()

So the test was failing and I put that as extra in the .toml file. Maybe as @DhanshreeA suggested we might need to invoke this from the above code, as a pytest fixture.

@Abellegese Abellegese self-assigned this Jan 9, 2025
@Abellegese
Copy link
Contributor

Will take care of this.

@GemmaTuron GemmaTuron moved this from On Hold to In Progress in Ersilia Model Hub Jan 10, 2025
@GemmaTuron
Copy link
Member Author

This should be tied to refactoring the test command to ensure it works in all Python env -- see #1484
@Abellegese can you tackle it in addition to the refactorings specified?

@Abellegese
Copy link
Contributor

Yes I pushed the changes a couple hours ago, will create a PR.

@Abellegese
Copy link
Contributor

Abellegese commented Jan 10, 2025

Hi @GemmaTuron but the log also shows that you had trouble installing scipt, I guess due to gfortran. To resolve this, you need to install a Fortran compiler. You can do this by installing the gfortran package, which is part of the GNU Compiler Collection. On Ubuntu or similar Linux distributions, you can install it using the following command:

sudo apt-get install gfortran

Updated: Also needed to remove the scipy version from the pyproject.toml, since the version specified in there is not supported by py3.12. So its also fixed to be *.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants