Nexus: Add PseudoSet class to improve pseudopotential handling for users. - #6043
Conversation
…g for POTCARs, add `__repr__` for `PseudoSet`
…on's limitations...
jtkrogel
left a comment
There was a problem hiding this comment.
Hold on this. Discussion needed when time permits later.
| cls.legacy_pseudos[label][code] = PseudoSet(pseudos=pseudos, code=code) | ||
| #end def _register_legacy_ppset | ||
|
|
||
| def __repr__(self) -> str: |
There was a problem hiding this comment.
Obviated by DevBase inheritance. Remove.
There was a problem hiding this comment.
The DevBase repr does not follow Python standards.
There was a problem hiding this comment.
Additionally, offline agreement was that the DevBase __repr__ and __str__ could be overridden if someone wanted to.
There was a problem hiding this comment.
Major classes inherit from DevBase in Nexus - this is fully intended to provide a consistent set of properties and behavior across the codebase. You already agreed to this with the streamlined DevBase update. Deviating repr behavior is only really warranted in "leaf" data types that approach simple/flat types rather than nested collections as in this case.
There was a problem hiding this comment.
I'm leaving __str__ intact, but keeping __repr__ as a form of prettyprint. Most people don't even interact with the repr function since print() defaults to an object's __str__ method.
There was a problem hiding this comment.
It's a start. The complete interface is there to allow people to navigate nested Nexus objects the same way one peruses a file system tree. Both functions are needed for this. Please make efforts to resist this urge down the line as it undermines the intentional construction at the foundation of the codebase.
…ng for overlapping file extensions
|
I want to note for the record that though this PR seems to add >2000 lines of code, if you remove tests and docstrings the actual |
|
@brockdyer03 Please fix the conflict when you have a moment |
|
Test this please |
prckent
left a comment
There was a problem hiding this comment.
Q. Is there a plan to add documentation of these capabilities?
I suggest to create an issue reminding us that this is needed. These user friendly features will only exist for users when e.g. they are in the docs with a small copyable example.
For starters, there are examples written into the docstrings of the code that demonstrate usage. But also, I do plan on writing extensive examples into an rST for users to see with copyable examples and common use cases. |
Proposed changes
This PR adds a class called
PseudoSetto Nexus that will eventually replace the existingppsetclass.PseudoSetoffers a more flexible and intuitive API for users that offers several advantages over the currentppset. Some notable advantages are:pseudo_dirinsettings).PseudoSet.from_dir().PseudoSet.from_mixed_dir().PseudoSet.get_Zeffs().ppsetwithPseudoSet._register_legacy_ppset().Example usage
I've included some examples here so you don't need to pore through the code to see how the functions work.
The examples here assume the following contents of
pseudo_dirPseudoSet.from_dir()PseudoSet.from_mixed_dir()What type(s) of changes does this code introduce?
Does this introduce a breaking change?
What systems has this change been tested on?
Laptop, Fedora Linux 43 (KDE Plasma Desktop Edition)
AMD Ryzen 7 PRO 7840U (8 cores, 16 logical processors)
Checklist