Install the chem extra in the jobs that exercise RDKit - #9
Merged
Conversation
Moving rdkit into the chem extra stopped CI installing it, so every test in tests/operators/test_drug_discovery.py and the drug-discovery, molnet and molecular-splitter suites failed with ModuleNotFoundError. The extra is right: installing the package should not pull a chemistry toolkit. The tests that cover that code still need it, so the two jobs that execute them ask for it explicitly. Scoped to those jobs rather than added everywhere. No test file imports rdkit at module level, verified by parsing each one, so collection is unaffected elsewhere; integration and e2e select no rdkit-dependent tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Moving
rdkitinto thechemextra stopped CI installing it, so the drug-discovery suites now fail onmainwithModuleNotFoundError: No module named 'rdkit'.The extra itself is right: installing the package should not pull a chemistry toolkit. But the tests that cover that code still need it.
Scope
Thirteen test files reference rdkit, spanning
tests/operators/,tests/benchmarks/,tests/sources/andtests/splitters/. Mapping jobs to the tests they execute, only two run any of them:unit_tests.[dev,test,genomics]→.[dev,test,genomics,chem]performance_tests.[dev,test,benchmark]→.[dev,test,benchmark,chem]integration_testsruns onlytests/integration/, which has no rdkit-dependent test, ande2e_testsselects-m e2e, which none of the thirteen carry.Collection is safe everywhere: every rdkit import in the test suite is inside a function or fixture, verified by parsing each of the thirteen files rather than by grep, so no job fails at import time.