pkienzle / periodictable

Extensible periodic table for python
http://periodictable.readthedocs.org
Other
147 stars 37 forks source link

Surprising result from formula() #14

Closed bjodah closed 8 months ago

bjodah commented 9 years ago

I would expect this to output 4, not 3:

>>> periodictable.formula('NH3 + H{+}').atoms[periodictable.H]
3

Py 2.7, periodictable 1.4.1

bjodah commented 9 years ago

So there is a distinction between ions and elements, I wrote a small utility function to get the elements of a formula:

from collections import defaultdict
def elements(formula):                                                                                        
    d = defaultdict(int)
    for atm, n in formula.atoms.items():
        try:
            d[atm.element] += n
        except AttributeError:
            d[atm] += n
    return d

Would it be of interest to have such a method in formulas.Formula? (I could open a PR)

pkienzle commented 9 years ago

The H+ is indexed as an ion:

>>> f = periodictable.formula('NH3 + H{+}')
>>> f.atoms
{H: 3, N: 1, H.ion[1]: 1}
>>> f.hill
formula('H3H{+}N')

It was easy enough to have ions treated as elements as far as counting is concerned:

>>> f = periodictable.formula('NH3 + D{+}')
>>> f.atoms
{H: 4, N: 1}
>>> f.isotopes
{H: 3, N: 1, H[2]: 1}
>>> f.ions
{H: 3, N: 1, H[2].ion[1]: 1}
>>> f.hill
formula('H4N')
>>> f.charge
1

I haven't checked it in yet. Before doing so I need to cross check the rest of the code base for whether f.atoms should change to f.isotopes or f.ions.

pkienzle commented 9 years ago

Looking at your alternative, leaving f.atoms alone and adding f.elements and f.isotopes is another way to go. This has the advantage of not changing the existing interface, but with the disadvantage of a more confusing naming scheme.

bjodah commented 9 years ago

I understand your dilemma between backwards compatibility and API cleanliness, nobody likes getting code broken..

On an unrelated note (didn't know if it's worth adding an issue for it): periodictable.formula('H{-}') raises a ValueError claiming -1 is not a valid charge for H, even though this is the hydride anion. Other physically sound examples raises this exception, e.g. He{+}.

pkienzle commented 9 years ago

I didn't document my source of oxidation states. I'll add a ticket saying some are missing.

pkienzle commented 8 months ago

I added count_elements() to accumulate across isotopes/ions.