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

Give two dict() in chemstry.Reaction() not work #218

Closed
Videatur opened this issue Nov 16, 2022 · 5 comments
Closed

Give two dict() in chemstry.Reaction() not work #218

Videatur opened this issue Nov 16, 2022 · 5 comments

Comments

@Videatur
Copy link

Documentation is clear on format Reaction(react,prod). React and prod are two dicts but is you try to give a dictionary object (like a variable) you'll see an error lambda for checking type.

Example

We try to take stoichiometry:

import chempy
react,prod = chempy.balance_stoichiometry({'H2', 'O2'}, {'H2O'})
print(chempy.chemistry.Reaction(dict(react), dict(prod)))

This will give:

line 577, in check_all_integral
    if v != type(v)(int(v)):
            ^^^^^^^^^^^^^^^
TypeError: Singleton.__init__.<locals>.<lambda>() takes 1 positional argument but 2 were given

This error will not occur if you pass result (we use dict() because belance create a OrderDict) dict in clear:

chempy.chemistry.Reaction({'H2': 2, 'O2': 1}, {'H2O': 2})

Is there a way for bypass? (store dict(react) or dict(prod) in another variable will give (of course) same error.

@bjodah
Copy link
Owner

bjodah commented Nov 16, 2022

Interesting, I think SymPy's Integer class might have changed:

>>> type(react['H2'])
sympy.core.numbers.Integer

or it has always been broken, I'm not sure. Either way there's plenty of things to improve on here (PRs most welcome).

But the short term work around is to perform no checks when creating Reaction:

>>> print(chempy.chemistry.Reaction(dict(react), dict(prod), checks=()))
2 H2 + O2 -> 2 H2O

or selective disable the "all_integral" check:

>>> print(chempy.chemistry.Reaction(dict(react), dict(prod), dont_check={"all_integral"}))
2 H2 + O2 -> 2 H2O

@Videatur
Copy link
Author

Interesting, I think SymPy's Integer class might have changed:

>>> type(react['H2'])
sympy.core.numbers.Integer

or it has always been broken, I'm not sure. Either way there's plenty of things to improve on here (PRs most welcome).

But the short term work around is to perform no checks when creating Reaction:

>>> print(chempy.chemistry.Reaction(dict(react), dict(prod), checks=()))
2 H2 + O2 -> 2 H2O

or selective disable the "all_integral" check:

>>> print(chempy.chemistry.Reaction(dict(react), dict(prod), dont_check={"all_integral"}))
2 H2 + O2 -> 2 H2O

Thank you so much, I'm gonna work on a pull to help you soon as can. I meantime this seem work fine.

@bjodah
Copy link
Owner

bjodah commented Nov 16, 2022

No stress! I reported the change in behavior in sympy/sympy#24268 too.

@montmorill
Copy link
Contributor

Could we just change chempy/chemistry.py#L577 from

if v != type(v)(int(v)):

to

if v != int(v) and v != type(v)(int(v)):

It does work.

@bjodah
Copy link
Owner

bjodah commented Sep 17, 2023

Sounds promising! would you mind opening a pull request?

@bjodah bjodah closed this as completed in 2ef4050 Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants