Fix/bug wrong input handling v2

[OlawumiSalaam]

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

Feedback from the previous PR has been addressed

Fixes #1539

[OlawumiSalaam]

@DhanshreeA I have pushed the changes. Please review

[OlawumiSalaam]

@DhanshreeA
I reviewed the code and traced the error, specifically 'NoneType' object has no attribute name. This error points to an attempt to access the name attribute of an object that is None.
Upon examining the code, I found that the error occurred during the invocation of the CliRunner().invoke(run_cmd(), ["-i", input_arg, "-o", str(output_arg)]) function. The root cause appears to be related to the run_cmd() function returning None, which led to a NoneType being passed where a valid Click command was expected.
The issue stems from run_cmd() not returning a proper Click command, which should be accessible and have the .name attribute. I had expected run_cmd() to return a callable Click command object (like @click.command), but it is returning None instead.
I have been trying to resolve this by ensuring that run_cmd() returns a valid Click command function, as intended. However, it seems that run_cmd() doesn't correctly register the command, leading to the error.

[GemmaTuron]

@OlawumiSalaam

Can you explain what is going on in this PR? It is indicated as fixing an already closed issue... Thanks and sorry I am trying to catch up with everything

[OlawumiSalaam]

@GemmaTuron
I had opened a PR before. @DhanshreeA requested for additional changes but merged the PR before I pushed the changes. So she asked me to open a new PR for the new changes she requested. That is why I have this new PR.

[GemmaTuron]

Can you also update this commit to see if it passes the tests? Thanks @OlawumiSalaam !

[OlawumiSalaam]

The PR is failing one of the test. Pytest specifically. I have tried to debug the test but I am not getting it.

[OlawumiSalaam]

@GemmaTuron
Error Identified: 'NoneType' object has no attribute 'name'.
Root Cause:
The error occurs during CliRunner().invoke(...) when run_cmd() returns None instead of a valid Click command.
The run_cmd() function isn’t properly returning a registered Click command (e.g., missing @click.command decorator or incorrect function registration).
Attempted Fixes:
Ensured run_cmd() returns a callable Click command object.
Debugged command registration logic but hit roadblocks in resolving the NoneType issue.

[miquelduranfrigola]

Thanks @OlawumiSalaam

I have quickly checked your modifications in this commit and I am not able to relate them to the error.

Can you rebase this repo and with the current Ersilia code, including your changes, and see if it passes the test?

On a separate note, I see the word "SMILES" being mentioned in your committed code. I would recommend that we do not put this word inside the main code of Ersilia. The reason is that, in the future, we will add other types of inputs beyond SMILES, so we want to keep it data type agnostic if possible.

[OlawumiSalaam]

@miquelduranfrigola Thanks. I will incorporate your feedback and update accordingly.
Thank you for your guidance.

[GemmaTuron]

Hi @OlawumiSalaam

I have updated the branch but I see the tests are yet not passing. I think the fix you propose is breaking the pytest when the inputs are being converted. Can you double check before moving onto the next task? Thanks!

[OlawumiSalaam]

@GemmaTuron @miquelduranfrigola @Abellegese

Summary of Debugging and Fixes

Issue Identified:
In the CI/PR environment, the RDKit library was not installed as seen here This caused the SMILES validation logic in the code to fall back to the asynchronous PubChem conversion method instead of using RDKit’s MolFromSmiles(). For one particular SMILES string,

(COc1cc(OC)c(/C=C/S(=O)(=O)Cc2ccc(OC)c(OP(=O)(O)O)c2)c(OC)c1)

the PubChem API did not return a valid InChIKey, leading to an UnprocessableInputError I implemented earlier to handle invalid input and causing the tests to fail.

Local vs. CI Environment:
Locally, I validated the SMILES string and it output a valid string

(ersilia) olawumi-salaam@DESKTOP-L7RDL5L:~/ersilia_internship/ersilia$ python3
Python 3.12.8 | packaged by Anaconda, Inc. | (main, Dec 11 2024, 16:31:09) [GCC 11.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from rdkit import Chem
>>> smiles = "COc1cc(OC)c(/C=C/S(=O)(=O)Cc2ccc(OC)c(OP(=O)(O)O)c2)c(OC)c1"
>>> mol = Chem.MolFromSmiles(smiles)
>>> print(mol)
<rdkit.Chem.rdchem.Mol object at 0x7f8d7849eb90>
>>> exit()

Locally, RDKit is installed, so the SMILES validation passes via RDKit, hence, the reason the test passed locally and the PR fail. So I proceeded to inspecting and debugging why the PR was failing the tests and discovered that in the CI environment, RDKit was missing, so the fallback using PubChem was invoked, and for this SMILES, it returned None ,and flagged a valid SMILES string as invalid.

Solution Implemented:
I updated the nox setup in our test environment to ensure RDKit is installed by adding the following line to our setup(session) function in noxfile.py:
session.run("conda", "install", "-c", "conda-forge", "rdkit", external=True)

This guarantees that RDKit is available in the CI environment, ensuring that the SMILES validation uses MolFromSmiles() as defined in ersilia/utils/identifiers/compound.py and passes correctly.

Outcome:
After adding the RDKit installation command, all tests passed in the CI environment, and the PR now meets the requirements.
Please review the PR

[miquelduranfrigola]

Thanks @OlawumiSalaam
@GemmaTuron @Abellegese I leave it to you guys to decide