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

Use PseudoPotentialData.jl instead of Artifacts #1020

Merged
merged 28 commits into from
Nov 30, 2024
Merged

Conversation

mfherbst
Copy link
Member

@mfherbst mfherbst commented Nov 18, 2024

  • Fix tests
  • Fix docs
  • Remove Artifacts.toml

@mfherbst mfherbst added the breaking Breaks public API and requires major version bump label Nov 18, 2024
@mfherbst mfherbst marked this pull request as ready for review November 20, 2024 09:13
@mfherbst
Copy link
Member Author

@antoine-levitt Any opinions on this ?

Essentially for pseudopotentials you can use PseudoPotentialData or a simple instead of specifying the full file name for each element. The main changes are visible in

  • docs/src/guide/tutorial.jl
  • docs/src/ecosystem/atomsbase.jl

Copy link
Member

@antoine-levitt antoine-levitt left a comment

Choose a reason for hiding this comment

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

Sure, that looks nice! There is this though "Warning: The current identifiers for the pseudopotential families is planned to be overhauled. This will be a breaking change, where the minor version of the package will be bumped." If the change is coming soon can't we wait until that's done to make the change in DFTK?

README.release_notes Outdated Show resolved Hide resolved
README.release_notes Outdated Show resolved Hide resolved
README.release_notes Outdated Show resolved Hide resolved
@mfherbst
Copy link
Member Author

Well that requires still some discussions with people from the Aiida world and some annoying manual changes I have no idea when I or someone will actually do it.

Lets merge now and have one breaking DFTK release instead of two in relatively short succession.

@mfherbst
Copy link
Member Author

Lets merge now and have one breaking DFTK release instead of two in relatively short succession.

I have an idea for a quick fix, which will avoid the changes in the naming. I'll do that before the merge.

@mfherbst mfherbst merged commit ce14127 into master Nov 30, 2024
5 of 8 checks passed
@mfherbst mfherbst deleted the add-pseudolibrary branch November 30, 2024 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaks public API and requires major version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants