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 flag write_site_properties = False in CifWriter for writing Structure.site_properties as _atom_site_{prop} #3550

Merged
merged 26 commits into from
Jan 14, 2024

Conversation

Andrew-S-Rosen
Copy link
Member

@Andrew-S-Rosen Andrew-S-Rosen commented Jan 13, 2024

Summary

Closes #3548 by adding a write_site_properties: bool = False keyword argument to CifWriter. Existing behavior is unchanged, but if write_site_properties = True, then the Structure object's site properties will be written out to the CIF file as atom site entries.

In principle, it would be nice to add a parser of custom site properties in CifParser, but I'll leave that to a separate PR.

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

@Andrew-S-Rosen
Copy link
Member Author

Andrew-S-Rosen commented Jan 13, 2024

@janosh: This is ready for review!

Edited because I solved the issue.

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 @Andrew-S-Rosen, nice addition! 👍

@janosh janosh enabled auto-merge (squash) January 14, 2024 15:38
@janosh janosh changed the title Allow for writing of Structure.site_properties as _atom_site_ flags in CifWriter Add flag write_site_properties = False in CifWriter for writing Structure.site_properties as _atom_site_{prop} Jan 14, 2024
@janosh janosh merged commit db20521 into materialsproject:master Jan 14, 2024
22 checks passed
@janosh janosh added enhancement A new feature or improvement to an existing one io Input/output functionality cif Crystallographic information file labels Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cif Crystallographic information file enhancement A new feature or improvement to an existing one io Input/output functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow for custom _atom_site_ fields in CifWriter
2 participants