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 - UserDict subclassing #731

Merged
merged 11 commits into from
May 15, 2024
Merged

Composition - UserDict subclassing #731

merged 11 commits into from
May 15, 2024

Conversation

SamG-01
Copy link
Contributor

@SamG-01 SamG-01 commented May 1, 2024

Changes the Composition class to subclass UserDict, allowing for direct key-value access of nuclei by string (e.g. comp["he4"]) or by Nucleus object (e.g. comp[Nucleus("he4")] or comp.X[Nucleus("he4")]. This also allows for dictionary operations such as for nuc, x in comp.items(): and len(comp). Also adds comp.A and comp.Z getters for dictionaries of the molar masses and charges.

@SamG-01 SamG-01 changed the title Small rate_collection.py changes Composition Magic Methods May 1, 2024
@yut23
Copy link
Collaborator

yut23 commented May 1, 2024

I really like __getitem__/__setitem__ being able to handle strings; that'll make interactive use a lot nicer. We can probably remove a lot of the boilerplate functions by inheriting from collections.UserDict.

@SamG-01 SamG-01 marked this pull request as draft May 1, 2024 21:50
@SamG-01
Copy link
Contributor Author

SamG-01 commented May 1, 2024

With this, certain methods like set_nuc will become obsolete. Should these be kept in the class for compatibility with existing code?

@SamG-01 SamG-01 changed the title Composition Magic Methods Composition - UserDict subclassing May 2, 2024
@SamG-01
Copy link
Contributor Author

SamG-01 commented May 2, 2024

In addition to subclassing UserDict, I also added a getter and setter for a comp.X property for backwards compatibility, as well as comp.A and comp.Z getters for dictionaries of the molar masses and charges (such that comp[n].A = comp.A[n]). The methods comp.get_nuclei() and comp.set_nuc(name, xval) are now superseded by list(comp.keys()) and comp[name] = xval respectively, but are left in for compatibility with existing code for the time being.

@SamG-01 SamG-01 marked this pull request as ready for review May 2, 2024 02:53
@yut23
Copy link
Collaborator

yut23 commented May 6, 2024

Can you add a quick test to make sure all the different indexing variations work the same? (comp[str], comp[Nucleus], comp.X[Nucleus])
I'll make a follow-up PR to update the usages elsewhere in the code.

Copy link
Collaborator

@yut23 yut23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pylint warnings are unrelated, from the new 3.2.0 version

@zingale zingale merged commit 6b82659 into pynucastro:main May 15, 2024
8 of 10 checks passed
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

Successfully merging this pull request may close these issues.

None yet

3 participants