-
Notifications
You must be signed in to change notification settings - Fork 81
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
Small docstring fix for sumo-dosplot
#241
Conversation
Also added a docstring fix for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
If there was any resource to work on Sumo, we could use Literal type annotations to make this kind of thing much clearer in the code. (And enforce them with Pydantic/Beartype so they don't need explicit checks.)
Looking back at this code with a few more years of experience, it would also probably be a good idea to refactor the Vasp/Questaal paths into an object interface and eliminate a lot of the logic. Ah, well.
I know this is a small change but do feel free to mention these things in the CHANGELOG and claim your credit. We can always tidy them together into a "minor maintenance" item or something before release if they get unwieldy. |
By the way, I see you did the work on
|
Just noticed that the docstring for
zero-energy
insumo-dosplot
was incorrect (duplication ofzero-line
which has different behaviour), so just a small update