Skip to content

BUG: fix setting zero magmoms #3179

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
merged 3 commits into from
Jul 24, 2023
Merged

Conversation

lbluque
Copy link
Contributor

@lbluque lbluque commented Jul 23, 2023

Summary

With the new changes that set the default Sepcies.spin to None, I fixed the logic for setting zero magmoms in SpacegroupAnalyzer to check for None values as well.

Checklist

  • Google format doc strings added. Check with ruff.
  • Type annotations included. Check with mypy.
  • Tests added for new features/fixes.
  • If applicable, new classes/functions/modules have duecredit @due.dcite decorators to reference relevant papers by DOI (example)

Tip: Install pre-commit hooks to auto-check types and linting before every commit:

pip install -U pre-commit
pre-commit install

Signed-off-by: lbluque <lbluque@berkeley.edu>
@shyuep
Copy link
Member

shyuep commented Jul 23, 2023

Do you mind adding a unittest for one mag structure to check for this? I am worried that someone might make changes in future that breaks this again.

@janosh janosh added fix Bug fix PRs analysis Concerning pymatgen.analysis magmoms Magnetism related labels Jul 24, 2023
@shyuep shyuep enabled auto-merge (squash) July 24, 2023 16:32
@shyuep
Copy link
Member

shyuep commented Jul 24, 2023

Thanks!

@shyuep shyuep merged commit 4c7e997 into materialsproject:master Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analysis Concerning pymatgen.analysis fix Bug fix PRs magmoms Magnetism related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants