-
Notifications
You must be signed in to change notification settings - Fork 2
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
Added dataclasses decorator to molecule class #114
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #114 +/- ##
===========================================
- Coverage 100.00% 99.73% -0.27%
===========================================
Files 16 16
Lines 751 754 +3
===========================================
+ Hits 751 752 +1
- Misses 0 2 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Any other changes that you want to make? If not, looks good to me, you can un-draft it and I'll merge it 👍 |
There's another way to enforce class attribute type using the attr package link. |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Seems to be working, the remaining errors are from the #119. To note, although it is not listed, the class still accepts variables in the order they are listed in the class. Most of the variables are init as None without typing, and adding a type check is as simple as adding @chmwzc please help do a quick scan of the class and add appropriate validators if needed, do note that the validators will check the default and return an error if there's type mismatch. Leave out the validators if there's multiple acceptable typing. |
|
@chmwzc |
Sure, but before that, it seems like you're allowing for all the attributes to be defined in |
That's an artifact of how dataclass works. It doesn't allow me to write attributes with defaults before variables without defaults. If I try to move those few initially defined attributes to the bottom, that messes with existing workflow. |
The class attributes of the Molecule class does not enforce types, run_pyscf also overrides the attributes, so the tuple is accepted without issues.
Shouldn't pass the ucc_ansatz due to typing issues though.
Regarding type enforcement, see link.
tl;dr is that enforcing type on class attributes during init is not recommended, if type enforcement is desired, we can follow the example in one of the answers.