Skip to content

Miscellaneous changes, including a change to how species are named. #2921

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.

Sign up for GitHub

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

Open
rwest wants to merge 2 commits into
base: main
Choose a base branch
from
+4 −3
Open

Miscellaneous changes, including a change to how species are named. #2921

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ffb90b2
Minor: Fix a Cython declaration in a Reaction method.
rwest Aug 2, 2024
b1d63ef
Fix species rename from thermo library for reaction-generated species
rwest Apr 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion rmgpy/reaction.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1447,7 +1447,7 @@ def is_balanced(self):
from rmgpy.molecule.element import element_list
from rmgpy.molecule.fragment import CuttingLabel, Fragment

cython.declare(reactant_elements=dict, product_elements=dict, molecule=Graph, atom=Vertex, element=Element,
cython.declare(reactant_elements=dict, product_elements=dict, molecule=Molecule, atom=Atom, element=Element,
reactants_net_charge=cython.int, products_net_charge=cython.int)
Comment on lines +1450 to 1451
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

After switching the declared types to Molecule/Atom, ensure those names are in scope in this module under both Python execution and Cython compilation. If they aren’t already imported at module scope, add the appropriate imports (or local imports in this function) to avoid NameError/compile-time failures.

Copilot uses AI. Check for mistakes.

reactant_elements = {}
Expand Down
5 changes: 3 additions & 2 deletions rmgpy/rmg/model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,10 +365,11 @@ def make_new_species(self, object, label="", reactive=True, check_existing=True,
spec.molecular_weight = Quantity(spec.molecule[0].get_molecular_weight() * 1000.0, "amu")

if generate_thermo:
self.generate_thermo(spec)
# Rename from thermo library label only if no user-provided label exists yet.
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

The PR description says rmgpy/display.py was updated for improved Jupyter/IPython detection, but that file isn’t included in the provided diff set. Either include the rmgpy/display.py change in this PR or update the PR description to match the actual changes.

Copilot uses AI. Check for mistakes.
self.generate_thermo(spec, rename=not bool(spec.label))
Comment on lines +368 to +369
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

The comment states the rename decision should be based on whether the user provided a label, but the condition is based on spec.label, which may already be populated from non-user sources earlier in make_new_species (e.g., copied from the input object). To make the behavior match the intent and stay stable if initialization changes, consider basing rename on the label argument (or a dedicated user_label_provided flag captured before mutating spec.label). Also, rename=not bool(spec.label) can be simplified to rename=not spec.label once the correct source-of-truth is chosen.

Copilot uses AI. Check for mistakes.

# If the species still does not have a label, set initial label as the SMILES
# This may change later after getting thermo in self.generate_thermo()
# (applies when generate_thermo is False, or when no library match was found)
if not spec.label:
spec.label = spec.smiles

Expand Down
Loading