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

Check Inputs to Trajectory. #3978

Merged
merged 5 commits into from
Aug 8, 2024

Conversation

CompRhys
Copy link
Contributor

@CompRhys CompRhys commented Aug 8, 2024

Summary

Checks the inputs to Trajectory following an issue encountered where get_positions() method can return more positions than sites depending on filters.

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)

@janosh janosh added enhancement A new feature or improvement to an existing one ux User experience core Pymatgen core labels Aug 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.

thanks @CompRhys! 👍

@janosh janosh merged commit a485c55 into materialsproject:master Aug 8, 2024
33 checks passed
"""
coords = np.asarray(coords)
Copy link
Member

Choose a reason for hiding this comment

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

for later reference, considered using np.atleast3d to normalize user input with insufficient dimensions but unfortunately when passed a 2d array, atleast3d inserts a singleton dim at the end rather than the start so probably more hassle than it's worth

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Pymatgen core enhancement A new feature or improvement to an existing one ux User experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants