-
Notifications
You must be signed in to change notification settings - Fork 876
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
Improve type annotations and comments for io.cif
#3820
Merged
janosh
merged 31 commits into
materialsproject:master
from
DanielYang59:add-types-for-io-cif
May 15, 2024
Merged
Improve type annotations and comments for io.cif
#3820
janosh
merged 31 commits into
materialsproject:master
from
DanielYang59:add-types-for-io-cif
May 15, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DanielYang59
changed the title
Add type annotations and tweak comments for
Improve type annotations and comments for May 12, 2024
io.cif
io.cif
@janosh. Could you please review this as well? Thanks! |
janosh
approved these changes
May 15, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @DanielYang59, this looks good! 👍
Thanks for reviewing! |
DanielYang59
added a commit
to DanielYang59/pymatgen
that referenced
this pull request
May 16, 2024
fix `core.operations` add types for site, mypy errors to fix remove no_type_check decorator move dunder methods to the top fix mypy errors remove debug tag improve `spectrum` finish `core.tensors` finish `xcfunc` fix hash of `SymmOp` finish `core.trajectory` Add types for `core.molecular_orbitals/operations/sites/spectrum/tensor/xcfunc` (materialsproject#3829) * fix `core.molecular_orbitals` * fix `core.operations` * add types for site, mypy errors to fix * remove no_type_check decorator * move dunder methods to the top * fix mypy errors * remove debug tag * improve `spectrum` * finish `core.tensors` * finish `xcfunc` * fix hash of `SymmOp` * add a missing type * Revert "fix hash of `SymmOp`" This reverts commit bf2a953. * fix some types in operations * "TEST": remove one unused var, relocate another * final tweak types * avoid hard-code class name * format tweaks * type tweak * fix unit test * replace all `[0:x]` with `[:x]` Guard `TYPE_CHECKING` only imports (materialsproject#3827) * flake8 --select=TYP001 * fix type import * format docstring to google style * fix more unguarded typecheck imports * format more docs to Google format * trigger CI (better var names) --------- Co-authored-by: Janosh Riebesell <janosh.riebesell@gmail.com> move structures out of util dir (materialsproject#3831) Add matgenb in the aux link section. Because apparently it is not "discoverable". Fixes materialsproject#3811. More updates. Move AIMS notebooks to matgenb. Add README in examples folder that point to proper resources. Improve type annotations and comments for `io.cif` (materialsproject#3820) * remove duplicate warning * type and docstring tweaks * add some types for io.cif * add comments and types * temp save of some formatting * revert functional change * revert unrelated changes * fix unit test * remove update that does nothing * relocate `sub_space_group` and `space_groups` to their usage * add more types * pre-commit auto-fixes * breaking: make parse_magmom/oxi_stats private * remove merge header * fix unit test * add more types * fix mypy errors * add a few spaces * remove if name ==main * simplify "check to see if" in comments * final tweaks * revise docstring * replace deprecated abc.abstractproperty * add missing doc strings and standardize existing * breaking: fix typo in method name orthongonalize_structure * revert accidental change in test_composition.py --------- Co-authored-by: Janosh Riebesell <janosh.riebesell@gmail.com>
janosh
added a commit
that referenced
this pull request
May 17, 2024
* relocate dunder methods to top for core.trajectory fix `core.operations` add types for site, mypy errors to fix remove no_type_check decorator move dunder methods to the top fix mypy errors remove debug tag improve `spectrum` finish `core.tensors` finish `xcfunc` fix hash of `SymmOp` finish `core.trajectory` Add types for `core.molecular_orbitals/operations/sites/spectrum/tensor/xcfunc` (#3829) * fix `core.molecular_orbitals` * fix `core.operations` * add types for site, mypy errors to fix * remove no_type_check decorator * move dunder methods to the top * fix mypy errors * remove debug tag * improve `spectrum` * finish `core.tensors` * finish `xcfunc` * fix hash of `SymmOp` * add a missing type * Revert "fix hash of `SymmOp`" This reverts commit bf2a953. * fix some types in operations * "TEST": remove one unused var, relocate another * final tweak types * avoid hard-code class name * format tweaks * type tweak * fix unit test * replace all `[0:x]` with `[:x]` Guard `TYPE_CHECKING` only imports (#3827) * flake8 --select=TYP001 * fix type import * format docstring to google style * fix more unguarded typecheck imports * format more docs to Google format * trigger CI (better var names) --------- Co-authored-by: Janosh Riebesell <janosh.riebesell@gmail.com> move structures out of util dir (#3831) Add matgenb in the aux link section. Because apparently it is not "discoverable". Fixes #3811. More updates. Move AIMS notebooks to matgenb. Add README in examples folder that point to proper resources. Improve type annotations and comments for `io.cif` (#3820) * remove duplicate warning * type and docstring tweaks * add some types for io.cif * add comments and types * temp save of some formatting * revert functional change * revert unrelated changes * fix unit test * remove update that does nothing * relocate `sub_space_group` and `space_groups` to their usage * add more types * pre-commit auto-fixes * breaking: make parse_magmom/oxi_stats private * remove merge header * fix unit test * add more types * fix mypy errors * add a few spaces * remove if name ==main * simplify "check to see if" in comments * final tweaks * revise docstring * replace deprecated abc.abstractproperty * add missing doc strings and standardize existing * breaking: fix typo in method name orthongonalize_structure * revert accidental change in test_composition.py --------- Co-authored-by: Janosh Riebesell <janosh.riebesell@gmail.com> * remove accidental change * fix mypy errors * fix mypy error * add some for core.units * rename `ArrayWithFloatWithUnit` to `ArrayWithUnit` in comment * fix mypy errors * tweak docstring for units * nest internal funcs for units * simplify module docstring * revert to accessing private attrib * fix unit test and revert support for `Unit` * use `str` over `Unit` * support unit as both str and Unit * tweak docstring example * improve IndexError messages * test_index_error --------- Co-authored-by: Janosh Riebesell <janosh.riebesell@gmail.com>
janosh
added a commit
to esoteric-ephemera/pymatgen
that referenced
this pull request
May 17, 2024
…3820) * remove duplicate warning * type and docstring tweaks * add some types for io.cif * add comments and types * temp save of some formatting * revert functional change * revert unrelated changes * fix unit test * remove update that does nothing * relocate `sub_space_group` and `space_groups` to their usage * add more types * pre-commit auto-fixes * breaking: make parse_magmom/oxi_stats private * remove merge header * fix unit test * add more types * fix mypy errors * add a few spaces * remove if name ==main * simplify "check to see if" in comments * final tweaks * revise docstring * replace deprecated abc.abstractproperty * add missing doc strings and standardize existing * breaking: fix typo in method name orthongonalize_structure * revert accidental change in test_composition.py --------- Co-authored-by: Janosh Riebesell <janosh.riebesell@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
io.cif
if __name__ == "__main__"
fromcore.libxcfunc
(legacy from another PR)Potentially Breaking Changes (very unlikely)
Make
parse_oxi_states
andparse_magmoms
ofCifParser
private methods. Rationale:_get_structure
method to parse corresponding properties from the file.Remove unnecessary
lattice
argument fromparse_magmoms
method (now private). Rationale:Lattice is None
, and could be done outside this method.pymatgen/pymatgen/io/cif.py
Lines 850 to 869 in 2e1c301