Skip to content
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

Composition raise ValueError if formula string is only numbers and spaces #3517

Merged
merged 2 commits into from
Dec 15, 2023

Conversation

janosh
Copy link
Member

@janosh janosh commented Dec 15, 2023

Controlled by new strict: bool = True parameter to _parse_formula.

@janosh janosh added breaking Breaking change ux User experience core Pymatgen core labels Dec 15, 2023
@janosh janosh force-pushed the composition-raise-on-bad-formula branch from 828c3b6 to d194bf7 Compare December 15, 2023 03:09
@janosh janosh enabled auto-merge (squash) December 15, 2023 03:14
@janosh janosh merged commit d5d750f into master Dec 15, 2023
22 checks passed
@janosh janosh deleted the composition-raise-on-bad-formula branch December 15, 2023 03:29
@CompRhys
Copy link
Contributor

CompRhys commented Dec 15, 2023

Doesn't catch the "NaN" -> float("NaN") edge case

>>> Composition(float("NaN"))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "pymatgen/pymatgen/core/composition.py", line 127, in __init__
    elem_map = dict(*args, **kwargs)  # type: ignore
TypeError: 'float' object is not iterable
>>> float("NaN")
nan

I was going to add another check:

from math import isnan
...
        if len(args) == 1 and isinstance(args[0], Composition):
            elem_map = args[0]
        elif len(args) == 1 and isinstance(args[0], str):
            elem_map = self._parse_formula(args[0])  # type: ignore
        elif len(args) == 1 and isnan(args[0]):
            raise ValueError("float('NaN') is not a valid Composition, did you mean str('NaN')?")
        else:
            elem_map = dict(*args, **kwargs)  # type: ignore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking change core Pymatgen core ux User experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants