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

Simplify case handling in lobster.inputs #3848

Merged
merged 65 commits into from
Jun 8, 2024

Conversation

DanielYang59
Copy link
Contributor

@DanielYang59 DanielYang59 commented May 30, 2024

Simplify case handling in lobster.inputs

  • [Potentially breaking] Rename class variable LISTKEYWORDS to LIST_KEYWORDS for consistency.

Use {lowercase: camalCase} to simplify key case handling, for the following methods:

Now lower-case keys are used internally, and Camel case keys are reserved in case of writing (good looking) output files.

  • magic dict methods
  • from_file
  • write_lobsterin: Also use it to recover the key case.
  • diff

TODOs

  • Update available lobsterin keywords

@DanielYang59 DanielYang59 changed the title Tweak lobster.inputs Simplify case handling in lobster.inputs May 30, 2024
@DanielYang59
Copy link
Contributor Author

DanielYang59 commented Jun 5, 2024

Do we have further concerns? @JaGeo @QuantumChemist If not I might ping Janosh for reviewing?

@JaGeo
Copy link
Member

JaGeo commented Jun 5, 2024

@DanielYang59 I don't have time at the moment. I would really appreciate that we merge this very carefully to avoid any breakages - especially also of Lobster workflows in atomate2.

@DanielYang59
Copy link
Contributor Author

Sure no rush at all. Let's leave it until you have time to review :)

@JaGeo
Copy link
Member

JaGeo commented Jun 5, 2024

Thanks @DanielYang59 for understanding. I really appreciate your work here but cannot keep up with the pace. Sorry!

@DanielYang59
Copy link
Contributor Author

All good, I'm not in a rush at all :) Just tagging it to make everything organized. Thanks!

@QuantumChemist
Copy link
Contributor

Do we have further concerns? @JaGeo @QuantumChemist If not I might ping Janosh for reviewing?

I don't see any more things that require changes. I could check if it breaks LobsterPy's functionality over the weekend (this or the next ones). Sorry, I'm also too busy to test the changes in depth atm.

@DanielYang59
Copy link
Contributor Author

DanielYang59 commented Jun 7, 2024

I don't see any more things that require changes. I could check if it breaks LobsterPy's functionality over the weekend (this or the next ones). Sorry, I'm also too busy to test the changes in depth atm.

No worries, that would be very helpful too :)

@JaGeo
Copy link
Member

JaGeo commented Jun 8, 2024

  • Run atomate2 and lobsterpy tests with Python 3.12
  • Run atomate2 and lobsterpy tests with Python 3.11

(I am working on it - it just takes a while. Afterwards, it is good to merge from my side.)

@JaGeo
Copy link
Member

JaGeo commented Jun 8, 2024

It's fine. The Lobster tests in atomate2 and LobsterPy tests are all passing. From my side, it is ready to merge.

@DanielYang59
Copy link
Contributor Author

Thanks for the test :) I would ping @janosh for final review.

@janosh janosh added io Input/output functionality lobster Lobster package (Local Orbital Basis Suite Towards Electronic-Structure Reconstruction) housekeeping Moving around or cleaning up old code/files labels Jun 8, 2024
Copy link
Member

@janosh janosh left a comment

Choose a reason for hiding this comment

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

lgtm, thanks @DanielYang59! 👍

@janosh janosh enabled auto-merge (squash) June 8, 2024 15:30
@janosh janosh merged commit f545d58 into materialsproject:master Jun 8, 2024
33 checks passed
@DanielYang59 DanielYang59 deleted the tweak-lobster branch June 9, 2024 01:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
housekeeping Moving around or cleaning up old code/files io Input/output functionality lobster Lobster package (Local Orbital Basis Suite Towards Electronic-Structure Reconstruction)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants