Skip to content

Replace hard coded class names, add types for io.vasp.sets #3863

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

Merged

Conversation

DanielYang59
Copy link
Contributor

@DanielYang59 DanielYang59 commented Jun 5, 2024

Summary

  • Improve types for io.vasp.sets and comment tweaks
  • Replace hard coded class names
  • Use deadline in deprecated decorator for easier tracking of deprecated functions

@DanielYang59 DanielYang59 marked this pull request as ready for review June 6, 2024 03:01
@DanielYang59 DanielYang59 requested a review from JaGeo as a code owner June 7, 2024 02:48
@DanielYang59 DanielYang59 requested a review from janosh June 7, 2024 03:11
@DanielYang59
Copy link
Contributor Author

@janosh Ready for review now, thanks for your time :)

@janosh janosh added io Input/output functionality linting Linting and quality assurance vasp Vienna Ab initio Simulation Package types Type all the things labels Jun 7, 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.

thanks @DanielYang59! nice work! 👍

@janosh janosh enabled auto-merge (squash) June 7, 2024 12:52
@janosh janosh merged commit 7c3ef9e into materialsproject:master Jun 7, 2024
33 checks passed
@DanielYang59 DanielYang59 deleted the replace-hard-coded-class-names branch June 7, 2024 13:03
@DanielYang59
Copy link
Contributor Author

Thanks for reviewing, and I really appreciate your merging this before splitting vasp.sets :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
io Input/output functionality linting Linting and quality assurance types Type all the things vasp Vienna Ab initio Simulation Package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants