Update from comments for form and mult #549
Conversation
| """Construct a string from Reaction Class. | ||
|
|
||
| :return: String from self | ||
| """ |
There was a problem hiding this comment.
Edit:
def __str__(self) -> str:
"""Convert a reaction class object to a string.
:return: The reaction class string
"""
|
|
||
| :return: _description_ | ||
| :rtype: _type_ | ||
| """ |
There was a problem hiding this comment.
Edit:
def __repr__(self) -> str:
"""Get a string literal describing the reaction class object.
:return: The reaction class string literal
"""
There was a problem hiding this comment.
Note: __repr__() and __str__() are essentially the same, except that the first one is supposed to represent the object, whereas the second one is supposed to be the result of "converting the object to a string".
There was a problem hiding this comment.
The first one gets called when you call repr(object), whereas the second one gets called when you call str(object).
There was a problem hiding this comment.
Example:
>>> print(repr("abc"))
'abc'
>>> print(str("abc"))
abc
There was a problem hiding this comment.
The way we are using __repr__() above, simply returning the string literal, isn't really the intended usage, and I don't think it's really necessary. But I am hesitant to take it out because this class is tied in with some ugly higher-level code that is very bug-prone. Eventually, I would like to get rid of these classes entirely.
| @@ -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.
To be consistent with the standard formatting that the linter wants, let's replace questions like these with condition al statements:
"""Whether this reaction class is reversible.
| """ | ||
| 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.
"""Whether this reaction class is bimolecular.
(See above)
| @@ -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.
"""Whether this reaction class requires a spin designation.
| @@ -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.
Should be Formula (throughout). You can do this using right-click and "Rename symbol" to update all instances.
There was a problem hiding this comment.
Otherwise, the edits below look good.
| @@ -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.
Looks good up to this point. A few more edits for the functions below this point:
def from_string(...) -> Formula:
...
def sorted_symbols(...) -> tuple[str, ...]:
...
def sort_vector(fml: Formula, symbs: Sequence[str] | None = None) -> tuple[int, ...]:
| 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 |
|
|
||
| from ._mult import spin as _spin | ||
|
|
||
|
|
||
| def high(rct_mults: tuple[float], prd_mults: tuple[float]) -> int: | ||
| def high(rct_mults: Sequence[float], prd_mults: Sequence[float]) -> int: |
| @@ -18,7 +19,7 @@ def high(rct_mults: tuple[float], prd_mults: tuple[float]) -> int: | |||
| return min(_high(rct_mults), _high(prd_mults)) | |||
|
|
|||
|
|
|||
| def low(rct_mults: tuple[float], prd_mults: tuple[float]) -> int: | |||
| def low(rct_mults: Sequence[float], prd_mults: Sequence[float]) -> int: | |||
No description provided.