Skip to content

fix: guard rdkit-dependent constants in atoms.py#441

Open
mulatta wants to merge 1 commit intoa-r-j:masterfrom
mulatta:fix-atoms-rdkit-guard
Open

fix: guard rdkit-dependent constants in atoms.py#441
mulatta wants to merge 1 commit intoa-r-j:masterfrom
mulatta:fix-atoms-rdkit-guard

Conversation

@mulatta
Copy link

@mulatta mulatta commented Feb 13, 2026

Reference Issues/PRs

None found. This is a previously unreported bug.

What does this implement/fix? Explain your changes

graphein/molecule/atoms.py defines module-level constants using Chem.rdchem.* enums (e.g. ALLOWED_HYBRIDIZATIONS, ALL_BOND_TYPES), but the existing try/except block only guards the import rdkit.
Chem statement. When rdkit is not installed, these constants raise NameError, making import graphein.molecule fail entirely.

This contradicts the design intent visible in other molecule submodules (graphs.py, utils.py) where
rdkit is treated as an optional dependency with a flag-based guard pattern.

Fix: Add a _HAS_RDKIT flag to the existing try/except and wrap rdkit-dependent constants in if _HAS_RDKIT: / else: block with empty fallbacks. Pure Python constants (BASE_ATOMS, EXTENDED_ATOMS,
ALLOWED_DEGREES, etc.) are unchanged.

What testing did you do to verify the changes in this PR?

  • Verified import graphein.molecule and import graphein.molecule.atoms succeed in a Python environment without rdkit installed
  • Ran black --check and isort --check-only on the changed file — both pass

Pull Request Checklist

  • Added a note about the modification or contribution to the ./CHANGELOG.md file (if applicable)
  • Added appropriate unit test functions in the ./graphein/tests/* directories (if applicable)
  • Modify documentation in the corresponding Jupyter Notebook under ./notebooks/ (if applicable)
  • Ran python -m py.test tests/ and make sure that all unit tests pass (for small modifications, it might be sufficient to only run the specific test file, e.g., python -m py.test tests/protein/test_graphs.py)
  • Checked for style issues by running black . and isort .

@sonarqubecloud
Copy link

@mulatta
Copy link
Author

mulatta commented Feb 13, 2026

it seems pre-commit failure is not associated with this diff.

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.

1 participant