-
Notifications
You must be signed in to change notification settings - Fork 17
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
Update from comments for form and mult #549
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,8 +12,8 @@ | |
|
||
- const | ||
- util | ||
- error | ||
- mult | ||
- error | ||
- mult | ||
- form | ||
- inchi_key | ||
- vmat | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,13 @@ | ||
""" Handle Full Reaction-Class designations for reactions that | ||
describe all meaningful attributes of the reaction required | ||
for electronic structure and kinetic calculations. | ||
"""Handle Full Reaction-Class designations for reactions that | ||
describe all meaningful attributes of the reaction required | ||
for electronic structure and kinetic calculations. | ||
""" | ||
import dataclasses | ||
import enum | ||
|
||
|
||
class ReactionClass(str, enum.Enum): | ||
"""Reaction class names""" | ||
"""Reaction class names.""" | ||
|
||
TRIVIAL = "trivial" | ||
# Unimolecular reactions | ||
|
@@ -24,17 +24,25 @@ class ReactionClass(str, enum.Enum): | |
SUBSTITUTION = "substitution" | ||
|
||
def __str__(self): | ||
"""Construct a string from Reaction Class. | ||
|
||
:return: String from self | ||
""" | ||
return self.value | ||
|
||
def __repr__(self): | ||
"""_summary_. | ||
|
||
:return: _description_ | ||
:rtype: _type_ | ||
""" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Edit:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The first one gets called when you call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Example:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The way we are using |
||
return repr(self.value) | ||
|
||
@classmethod | ||
def reverse(cls, value: str): | ||
"""Get the class for the reverse of a reaction | ||
"""Get the class for the reverse of a reaction. | ||
|
||
:param value: A reaction class | ||
:type value: str | ||
:return: The class of the reverse reaction | ||
:rtype: ReactionClass | ||
""" | ||
|
@@ -57,23 +65,19 @@ def reverse(cls, value: str): | |
|
||
@classmethod | ||
def is_reversible(cls, value: str) -> bool: | ||
"""Is this reaction class reversible? | ||
"""Is this reaction class reversible?. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be consistent with the standard formatting that the linter wants, let's replace questions like these with condition al statements:
|
||
|
||
:param value: A reaction class | ||
:type value: str | ||
:return: `True` if it is, `False` if it isn't | ||
:rtype: bool | ||
""" | ||
return cls.reverse(value) is not None | ||
|
||
@classmethod | ||
def is_bimolecular(cls, value: str) -> bool: | ||
"""Is this reaction class bimolecular? | ||
"""Is this reaction class bimolecular?. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
(See above) |
||
|
||
:param value: The reaction class | ||
:type value: str | ||
:return: `True` if it is, `False` if it isn't | ||
:rtype: bool | ||
""" | ||
bimol_classes = ( | ||
cls.HYDROGEN_ABSTRACTION, | ||
|
@@ -86,12 +90,10 @@ def is_bimolecular(cls, value: str) -> bool: | |
|
||
@classmethod | ||
def requires_spin_designation(cls, value: str) -> bool: | ||
"""Is this a reaction class that requires a spin designation? | ||
"""Is this a reaction class that requires a spin designation?. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
:param value: The reaction class | ||
:type value: str | ||
:return: `True` if it is, `False` if it isn't | ||
:rtype: bool | ||
""" | ||
need_spin_classes = ( | ||
cls.HYDROGEN_ABSTRACTION, # AVC: Why is this in here?? | ||
|
@@ -101,53 +103,55 @@ def requires_spin_designation(cls, value: str) -> bool: | |
|
||
@classmethod | ||
def is_defined(cls, value: str) -> bool: | ||
"""Is this reaction class defined? | ||
"""Is this reaction class defined?. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
:param value: The reaction class | ||
:type value: str | ||
:return: `True` if it is, `False` if it isn't | ||
:rtype: bool | ||
""" | ||
return str(value) in list(cls) | ||
|
||
|
||
class ReactionSpin(str, enum.Enum): | ||
"""reaction spin types""" | ||
"""reaction spin types.""" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's capitalize the start of these docstrings: |
||
|
||
LOW = "low-spin" | ||
HIGH = "high-spin" | ||
NONE = "unspecified spin" | ||
|
||
def __str__(self): | ||
"""_summary_. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
:return: _description_ | ||
:rtype: _type_ | ||
""" | ||
return self.value | ||
|
||
def __repr__(self): | ||
"""_summary_. | ||
|
||
:return: _description_ | ||
:rtype: _type_ | ||
""" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
return repr(self.value) | ||
|
||
@classmethod | ||
def is_defined(cls, value: str) -> bool: | ||
"""Check whether a reaction spin type is defined | ||
"""Check whether a reaction spin type is defined. | ||
|
||
:param value: The value to check for | ||
:type value: str | ||
:return: `True` if it does, `False` if it doesn't | ||
:rtype: bool | ||
""" | ||
return value in list(cls) | ||
|
||
|
||
@dataclasses.dataclass | ||
class ReactionInfo: | ||
"""General information about a reaction | ||
"""General information about a reaction. | ||
|
||
:param class_: The class name of the reaction | ||
:type class_: ReactionClass | ||
:param spin_: The spin-type of the reaction (low or high) | ||
:type spin_: ReactionSpin | ||
:param is_rad_rad: Whether this is a radical-radical reaction | ||
:type is_rad_rad: bool | ||
:param is_isc: Whether this is an intersystem crossing | ||
:type is_isc: bool | ||
""" | ||
|
||
class_: ReactionClass | ||
|
@@ -156,7 +160,7 @@ class ReactionInfo: | |
is_isc: bool = False | ||
|
||
def __str__(self) -> str: | ||
"""Generate a string representation of the reaction information""" | ||
"""Generate a string representation of the reaction information.""" | ||
parts = [] | ||
|
||
if self.is_radical_radical(): | ||
|
@@ -172,47 +176,47 @@ def __str__(self) -> str: | |
return " ".join(map(str, parts)) | ||
|
||
def __repr__(self) -> str: | ||
"""Generate a string representation of the reaction information""" | ||
"""Generate a string representation of the reaction information.""" | ||
return str(self) | ||
|
||
def string(self) -> str: | ||
"""Get a string representation of the reaction""" | ||
"""Get a string representation of the reaction.""" | ||
return str(self) | ||
|
||
def reaction_class(self) -> ReactionClass: | ||
"""Get the reaction class name""" | ||
"""Get the reaction class name.""" | ||
return ReactionClass(self.class_) | ||
|
||
def reaction_spin(self) -> ReactionSpin: | ||
"""Get the reaction spin type""" | ||
"""Get the reaction spin type.""" | ||
return ReactionSpin(self.spin) | ||
|
||
def is_radical_radical(self) -> bool: | ||
"""Is this a radical radical reaction?""" | ||
"""Is this a radical radical reaction?.""" | ||
return self.is_rad_rad | ||
|
||
def is_intersystem_crossing(self) -> bool: | ||
"""Is this an intersystem crossing?""" | ||
"""Is this an intersystem crossing?.""" | ||
return self.is_isc | ||
|
||
def is_barrierless(self) -> bool: | ||
"""Is this a barrierless reaction?""" | ||
"""Is this a barrierless reaction?.""" | ||
return self.is_radical_radical() and not self.is_high_spin() | ||
|
||
def is_low_spin(self) -> bool: | ||
"""Is this a low-spin reaction?""" | ||
"""Is this a low-spin reaction?.""" | ||
return self.reaction_spin() == ReactionSpin.LOW | ||
|
||
def is_high_spin(self) -> bool: | ||
"""Is this a high-spin reaction?""" | ||
"""Is this a high-spin reaction?.""" | ||
return self.reaction_spin() == ReactionSpin.HIGH | ||
|
||
def has_no_spin_designation(self) -> bool: | ||
"""Is this the only possible spin-state?""" | ||
"""Is this the only possible spin-state?.""" | ||
return self.reaction_spin() == ReactionSpin.NONE | ||
|
||
def requires_spin_designation(self) -> bool: | ||
"""Is a spin designation required for this reaction?""" | ||
"""Is a spin designation required for this reaction?.""" | ||
spin_req_classes = ( | ||
ReactionClass.HYDROGEN_ABSTRACTION, # AVC: Why is this in here?? | ||
ReactionClass.ADDITION, | ||
|
@@ -225,7 +229,7 @@ def requires_spin_designation(self) -> bool: | |
|
||
def requires_well_description(self) -> bool: | ||
"""Determine if a reaction is appropriately described by the presence of | ||
entrance- or exit-channel van der Waals wells | ||
entrance- or exit-channel van der Waals wells. | ||
""" | ||
well_classes = ( | ||
ReactionClass.HYDROGEN_ABSTRACTION, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,9 +18,10 @@ | |
STOICH = pp.Opt(pp.Literal("*") | ppc.integer).setParseAction(lambda x: x if x else [1]) | ||
ATOM_COUNT = pp.Group(SYMBOL + STOICH) | ||
FORMULA = pp.OneOrMore(ATOM_COUNT) | ||
Formul = dict[str, int] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Otherwise, the edits below look good. |
||
|
||
|
||
def electron_count(fml: dict[str:int]) -> int: | ||
def electron_count(fml: Formul) -> int: | ||
"""Count the number of electrons for the atoms in a molecular formula. | ||
|
||
:param fml: Stochiometric chemical formula | ||
|
@@ -36,7 +37,7 @@ def electron_count(fml: dict[str:int]) -> int: | |
return elec_count | ||
|
||
|
||
def atom_count(fml: dict[str:int]) -> int: | ||
def atom_count(fml: Formul) -> int: | ||
"""Count the number of atoms in this molecular formula. | ||
|
||
:param fml: Stochiometric chemical formula | ||
|
@@ -47,7 +48,7 @@ def atom_count(fml: dict[str:int]) -> int: | |
return sum(fml.values()) | ||
|
||
|
||
def heavy_atom_count(fml: dict[str:int]) -> int: | ||
def heavy_atom_count(fml: Formul) -> int: | ||
"""Count the number of heavy atoms in this molecular formula. | ||
|
||
:param fml: Stochiometric chemical formula | ||
|
@@ -58,7 +59,7 @@ def heavy_atom_count(fml: dict[str:int]) -> int: | |
return sum(fml.values()) | ||
|
||
|
||
def element_count(fml: dict[str:int], symb: str) -> int: | ||
def element_count(fml: Formul, symb: str) -> int: | ||
"""Count the number of a given element in this molecular formula. | ||
|
||
:param fml: Stochiometric chemical formula | ||
|
@@ -98,7 +99,7 @@ def match(fml1: dict[str, int], fml2: dict[str, int]) -> bool: | |
return fml1 == fml2 | ||
|
||
|
||
def add_element(fml: dict[str:int], symb: str, num: int = 1) -> dict[str:int]: | ||
def add_element(fml: Formul, symb: str, num: int = 1) -> Formul: | ||
"""Add or subtract (if num < 0) this element from the molecular formula. | ||
|
||
:param fml: Stochiometric chemical formula | ||
|
@@ -121,7 +122,7 @@ def add_element(fml: dict[str:int], symb: str, num: int = 1) -> dict[str:int]: | |
return fml | ||
|
||
|
||
def join(fml1: dict[str:int], fml2: dict[str:int]) -> int: | ||
def join(fml1: Formul, fml2: Formul) -> int: | ||
"""Join two formulas together. | ||
|
||
:param fml1: Stochiometric chemical formula 1 | ||
|
@@ -135,7 +136,7 @@ def join(fml1: dict[str:int], fml2: dict[str:int]) -> int: | |
return fml | ||
|
||
|
||
def join_sequence(fmls: dict[str:int]) -> int: | ||
def join_sequence(fmls: Formul) -> int: | ||
"""Join a sequence of formulas together. | ||
|
||
:param fml: Stochiometric chemical formula | ||
|
@@ -154,7 +155,7 @@ def sorted_symbols_in_sequence(fmls: list[dict]) -> list[dict]: | |
|
||
|
||
# Str<->Dict Converters | ||
def string(fml: dict[str:int], hyd: bool = True) -> str: | ||
def string(fml: Formul, hyd: bool = True) -> str: | ||
"""Convert formula dictionary to formula string in the Hill convention. | ||
Resultant string is identical to InChI formula string. | ||
|
||
|
@@ -173,7 +174,7 @@ def string(fml: dict[str:int], hyd: bool = True) -> str: | |
return fml_str | ||
|
||
|
||
def string2(fml: dict[str:int]) -> str: | ||
def string2(fml: Formul) -> str: | ||
"""Convert formula dictionary to formula string that includes 1s in when | ||
there is only one atom. | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks good up to this point. A few more edits for the functions below this point:
|
||
|
@@ -281,7 +282,7 @@ def sort_vector(fml: str, symbs: list[str] | None = None): | |
return vec | ||
|
||
|
||
def _is_standard(fml: dict[str:int]) -> bool: | ||
def _is_standard(fml: Formul) -> bool: | ||
"""Assess if the formula conforms to the standard form. | ||
|
||
:param fml: stochiometric chemical formula | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,12 @@ | ||
"""reaction formulae.""" | ||
import itertools | ||
|
||
from ._form import add_element, join_sequence | ||
from _collections_abc import Sequence | ||
|
||
from ._form import Formul, add_element, join_sequence | ||
|
||
def is_valid_reaction(rct_fmls: list[str], prd_fmls: list[str]) -> bool: | ||
|
||
def is_valid_reaction(rct_fmls: Sequence[Formul], prd_fmls: Sequence[Formul]) -> bool: | ||
"""Use the formula to see if a reaction preserves stoichiometry. | ||
|
||
:param rct_fmls: stoichiometries of the reactants | ||
|
@@ -14,14 +16,16 @@ def is_valid_reaction(rct_fmls: list[str], prd_fmls: list[str]) -> bool: | |
return join_sequence(rct_fmls) == join_sequence(prd_fmls) | ||
|
||
|
||
def argsort_hydrogen_abstraction(rct_fmls: list[str], prd_fmls: list[str]) -> bool: | ||
"""Generates the indices which allows the reactants and products of | ||
def argsort_hydrogen_abstraction( | ||
rct_fmls: Sequence[Formul], prd_fmls: Sequence[Formul] | ||
) -> tuple[tuple[int, int], tuple[int, int]] | None: | ||
"""Generate the indices which allows the reactants and products of | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks good. |
||
a hydrogen abstraction reaction can be sorted as RH + Q => R + QH. | ||
|
||
:param rct_fmls: stoichiometries of the reactants | ||
:param prd_fmls: stoichiometries of the products | ||
:return: Indices to sort reaction to R + QH | ||
""" # noqa: D401 | ||
""" | ||
rxn_idxs = None | ||
if len(rct_fmls) == len(prd_fmls) == 2: | ||
for idxs1, idxs2 in itertools.product( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Edit: