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

Add an input arg check for Kpoints.automatic_density_by_lengths #3299

Merged
merged 5 commits into from
Sep 3, 2023
Merged

Add an input arg check for Kpoints.automatic_density_by_lengths #3299

merged 5 commits into from
Sep 3, 2023

Conversation

Andrew-S-Rosen
Copy link
Member

Summary

The function Kpoints.automatic_density_by_length() takes in a sequence of values, e.g. [50, 50, 1], to set the k-point density for each dimension. However, the function does not raise any kind of error if a sequence of dimensions != 3 is supplied.

This PR adds a ValueError if the len is != 3, along with a test for it.

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

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 @arosen93!

@janosh janosh merged commit 4bade9c into materialsproject:master Sep 3, 2023
@Andrew-S-Rosen Andrew-S-Rosen deleted the kpts_error branch September 3, 2023 06:07
@janosh janosh added tests Issues with or changes to the pymatgen test suite ux User experience labels Sep 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Issues with or changes to the pymatgen test suite ux User experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants