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

Removing Custom Quantity Serialization from as_dict in Solution.py #180

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

SuixiongTay
Copy link
Contributor

Summary

Removed the custom code in Solution.as_dict that handles the serialization of Quantity objects.

Details:

  • Removed the custom Quantity code from as_dict.
  • Updated the pyproject.toml to bump the monty version from v2024.7.12 to v2024.7.29.

@rkingsbury
Copy link
Member

Thank you @SuixiongTay ! Nice clean PR.

I am puzzled by why the tests are failing. The failure mode is consistent

 src/pyEQL/solution.py:2368: in from_dict
    orig_volume = ureg.Quantity(d["volume"])
        cls        = <class 'pyEQL.solution.Solution'>
        d          = {'balance_charge': None, 'database': {'@class': 'JSONStore', '@module': 'maggma.stores.mongolike', '@version': '0.69.3', 'encoding': 'utf8', ...}, 'default_diffusion_coeff': 1.6106e-09, 'engine': 'native', ...}
/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/pint/facets/plain/quantity.py:229: in __new__
    magnitude = _to_magnitude(
        cls        = <class 'pint.Quantity'>
        inst       = <[RecursionError('maximum recursion depth exceeded while calling a Python object') raised in repr()] Quantity object at 0x116e8fbe0>
        units      = <UnitsContainer({})>
        value      = {'@class': 'Quantity', '@module': 'pint', '@version': '0.24.3', 'data': '2 l'}
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

value = {'@class': 'Quantity', '@module': 'pint', '@version': '0.24.3', 'data': '2 l'}
force_ndarray = False, force_ndarray_like = False

    def _to_magnitude(value, force_ndarray=False, force_ndarray_like=False):
        if isinstance(value, (dict, bool)) or value is None:
>           raise TypeError(f"Invalid magnitude for Quantity: {value!r}")
E           TypeError: Invalid magnitude for Quantity: {'@module': 'pint', '@class': 'Quantity', 'data': '2 l', '@version': '0.24.3'}

force_ndarray = False
force_ndarray_like = False
value      = {'@class': 'Quantity', '@module': 'pint', '@version': '0.24.3', 'data': '2 l'}

/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/pint/compat.py:104: TypeError

It seems as if the entire dict (from Quantity.as_dict()) is getting passed back as an argument to re-create the Quantity, but I can't figure out why that's happening. You can see from the stacktrace (the E line) that the value is {'@module': 'pint', '@class': 'Quantity', 'data': '2 l', '@version': '0.24.3'} whereas I think it should just be '2 l'.

This is going to take some troubleshooting. This is the PR that added Quantity support to monty, and having read over it again, I don't think this is a monty problem but rather something about how the data is getting passed around in pyEQL (although I could be wrong).

@rkingsbury rkingsbury added dependencies Pull requests that update a dependency file housekeeping labels Aug 26, 2024
@rkingsbury
Copy link
Member

For troubleshooting I suggest the following steps:

  1. Manually create a pint Quantity, then dumpfn and loadfn it to confirm that works (this is already tested in the monty PR so it should work.
  2. Manually create a simple solution of NaCl or something. Run as_dict and inspect the output. Anywhere there's a pint Quantity, you should see it replaced by a dict that looks like this {'@class': 'Quantity', '@module': 'pint', '@version': '0.24.3', 'data': '2 l'}
  3. Save that dictionary to a variable d. See if you can run Solution.from_dict(d) successfully.
  4. If that succeeds, then dumpfn the solution (or use .to_file() to create a json. Inspect the .json file output manually and see what the Quantity objects look like
  5. Finally, call from_file() on the json file and see if it work.s

This should help us identify which part of the code is causing the failure.

@rkingsbury
Copy link
Member

Closes #159

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file housekeeping
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants