Fix circular import of SymmOp
#4357
Merged
Conversation
This file contains hidden or 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
|
No that was an import I used for debugging and forgot to delete, apologies for leaving it in there. |
|
No worries at all, it simply means we're missing a test for this :D |
benrich37
pushed a commit
to benrich37/pymatgen
that referenced
this pull request
Apr 14, 2025
benrich37
pushed a commit
to benrich37/pymatgen
that referenced
this pull request
Apr 14, 2025
shyuep
added a commit
that referenced
this pull request
May 8, 2025
* Adding "tests/files/io/jdftx/*" to codespell exception * Changing shortening of "structure" from "struc" to "struct" as no longer an excepted word by codespell * adding option for JDFTXOutfileSlice's to be initialized as None if there are issues initializing any particular slice. The default for this option is False for now * Converting JElSteps(s) docustrings to google style, privatizing non-user methods and attributes for JElStep(s) (this includes initialization methods as these objects are not supposed to be created by users) + using/updating Structure site_properties for charges and magnetic_moments (now set as properties instead of attributes) * fixing incomplete function renaming refactoring * fixing incomplete function renaming refactoring * privatizing JOutStructure initialization helpers, converting docustrings to google style * privatizing JOutStructures initialization helpers, converting docustrings to google style * privatizing JOutStructures initialization helpers, converting docustrings to google style * privatizing JDFTXOutfileSlice initialization helpers, converting docustrings to google style * converting docustrings to google style * incomplete renaming fixes, protection from setting magnetic moments and charges to None * privitizing jdftx io utils module * privitizing jdftx io utils module * docustring update to google style * docustring update to google style * site_properties behavior correction * syncing unit test for testing valences from feature-ptable branch * renaming test file * separating _utils.py into _input_utils.py and _output_utils.py * updating module docustring for _output_utils * input/output utils testing separation * sourcing changes made in feature-jdftx-inputs * renaming test file to matching renaming of "JEiters" to "JElSteps" * Moving test files to directories with more descriptive names, removing test files not being used * moving multi_(has/get)_attr from output utils to inputs.py (only used in inputs.py) and made them both private * changing enumeration of valence electrons (the `valences` method does not exactly what I thought it did. I still think its a helpful function to have but not what I need here) * explicit testing of "initial-magnetic-moments" * added "to_dict" method to JDFTXOutfile * adding `JDFTXOutfileSlice` to "test_dict_repr" in "test_repr_out.py" * removing fatal error from retrieving t_s from a JOutStructure that is None * Correction for ruff - PLC0206 (Extracting value from dictionary without calling `.items()`) - This does not appear in my local pre-commit (which I believe is running correctly) but started causing the "lint" action to fail starting around 11/22 (along with other failures from unrelated pre-existing parts of pymatgen) * changing the previous "as_dict" method to be private, making a public "as_dict" method that passes through the __setitem__ method so that the values within the dictionary fed to "as_dict" can be as flexible as the values set explicitly by the user (also made minor updates to __setitem__ to handle setting multiple values for repeatable tags better) * removing __getattr__ and writing allocating everything in the __postinit__ * replaced all properties within JDFTXOutfile and JDFTXOutfileSlice as initialized attributes and fixed hidden errors revealed by doing so * removing review requests for files with anticipated changes coming up * replaced all properties within JElStep(s) and JOutStructure as initialized attributes (skipped for charges and magnetic_moments as the setter/getter methods are important as the user might interact with this object) * Cleaning up __post_init__ methods to iterate over a list of variables for more easier reading * removing now unneeded __getattr__ from JDFTXOutfileSlice, no longer have properties in JDFTXOutfile * consistent "to_dict" methods * Removing references to properties * adding **kwargs for JOutStructure for flexibility * "from_calc_dir" init method for JDFTXOutfile * Partial cleanup of tests * Partial cleanup of tests * Partial cleanup of tests * parsing for necessary data to interact with eigenvals + projections for pDOS/pCOHP analysis * updates to necessary stored metadata and example files for testing * Adding "structure" attribute to JOutStructure for convenient way to avoid any issues with using a JOutStructure in place of a Structure, adding eigenvals and bandProjections to JDFTXOutputs * Changed trajectory to pull from Structure objects (instead of JOutStructure objects), added a "constant_lattice" bool attribute to JDFTXOutfileSlice for creating a Trajectory, changed the "_get_trajectory" method for JDFTXOutfile to instead concatenate the "trajectory" attributes of the JDFTXOutfileSlice's * None-type protection for setting constant_lattice in JDFTXOutfileSlice, removing test for ValueError no longer raised in _set_trajectory in JDFTXOutfileSlice * oversight - this should've always been a Structure * Cleanup of inputs - homogenized wording in error messages, added value validation methods to AbstractTag for validations that are called multiple times, cleaned up cluttered tests and added comments to make it more clear what is being tested * populating attributes taken from `jstrucs` for `JDFTXOutfileSlice` within a for-loop now, adding "forces" as an attribute passed upstread to JDFTXOutfile * Removing commented out code (all attributes set within the above for loop) * fixes to consistent ion coordinate units - added missing bohr_to_ang when reading initial coordinates in joustructures.py (_get_initial_coords), added missing 1/bohr_to_ang when creating a JDFTXInfile from a pmg Structure, added specification of coords-type when creating JDFTXInfile from a pmg Structure, added keyword argument for from_structure to store ion position tags as fractional or cartesian coordinates * fixes for formatting on TagContainer writing with linebreak_nth_entry * removing commented out code * activating none_slice_on_error behavior for all outfile slices EXCEPT the final slice * fixed cartesian coords in from_jdftxinfile method and added Etot to parsed energy types * fixed oversight on how 'none_slice_bools' are set on the 'none_slice_on_error is None' condition * pre-commit auto-fixes * Changing all "zopen" to just "open" due to an error raised when monty.io tries to raise an "EncondingWarning", changing the dump files dir to be a fixture that creates itself, yields the path, and then removes itself (parts after a "yield" only run once the test using the fixture has finished) * Storing a list of the orbital labels instead of a dictionary of orbitals available per ion-type, adding a value check for the bandProjections, improving docustring for JDFTXOutputs * optional "outfile_name" argument for "from_calc_dir" for JDFTXOutputs to allow for initialization on calc_dirs with ambiguous out file identity (ie multiple files with the .out suffix) * Oversight that causes ionic positions from previous strucure to be passed incorrectly * Oversight that causes ionic positions from previous strucure to be passed incorrectly * Reverting old changes to sync with main fork * Removing unused _input_utils * Adding fixes to mistakes found by jacob * Adding fixes to mistakes found by jacob * The value for a JOutStructures should be the most recent non-None value (some values will not be logged again for parsing if they are not updated at an optimization step) * The value for a JOutStructures should be the most recent non-None value (some values will not be logged again for parsing if they are not updated at an optimization step) * The value for a JDFTx output object containing slices should be the most recent non-None value (some values will not be logged again for parsing if they are not updated at an optimization step) * The value for a JDFTx output object containing slices should be the most recent non-None value (some values will not be logged again for parsing if they are not updated at an optimization step) * protection since an outfile slice might beinitialized as None * renaming get_colon_var_t1 to get_colon_val * Moving functions reading "input" structure from the outfile from joutstructures.py to jdftxoutfileslice.py, making the "init_struc" an initialization argument for a JOutStructures instead of something constructed during initialization, reducing the number of instances where object indexes are referenced and increasing the number of attributes explicitly set as None to avoid later references to unset attributes * typo * Error if outfile slice does not even contain the inputs preamble for the structure, removing "initial_structure" from the default behavior "JOutStructures attribute from slices" list * oversight causing the initial value to be set as the parent value, not the most recent value * missing indexing fix * Fixing oversight in input file printing causing multiline tags to miss the indent on the first line of their values (thank you Jacob for catching!), adding t_s as an attribute to be inherited in the _<x>_atr_from_<y>"loops instead, fixing _get_t_s from returning None when self.t_s has a non-None value, changing known value in outfile tests from 'None' to None, removing test_imports tests for now * Fixing "count" enumeration in when gathering subtag value strings in TagContainer.write so that repeating subtags may be broken up into seperate lines when requested in the TagContainer linebreak_nth_entry (or multiline_tag) * phasing out TagContainer.multiline_tag since its redundant to TagContainer.linebreath_nth_entry == 1, fixing oversight for linebreath_nth_entry == 1 case which was causing new lines from getting an indent * fix dump-name tag formatting * Adding missing options from the func_x_options list * Adding missing options from the func_x_options list * Fix for selective_dynamics for the JOutStructure class - now saves a list[list[int]] (forced into 3xN if only one flag for ion) and saves within the site_properties instead of as a class variable as per pymatgen convention * Some simply mypy fixes. But there are too many to deal with. * Add .python-version * Seems like micromamba doesn't like python version. * COmment out mypy checks for now. * Add comment on how to update PT data. * Properly use dependency groups for uv. * See if adding a uv sync solvs the problem. * Add back ci category. * Add prototypes to extras. * Updated Potentials Class in FEFF io to consider radius (#4334) * Changed the Potentials class to consider the same radius that is used in the Atoms class. This is necessary to avoid a situation where the radius is small enough to only have a subset of the unique elements in a structure, but all the elements have potentials defined. This will cause FEFF to fail when run. * pre-commit auto-fixes * added tests and fixed undefined radius in Feff Potential to be consistent with previous behavior --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Shyue Ping Ong <[email protected]> * Rename workflows. * Upgrade matgl. Remove dependency on chgnet since this has now been moved to within matgl, which is easier. * Add ubuntu arm build for wheels in pymatgen. * remove data high (#4341) * fix Mn ionic radii (#4342) * Fix workflow for Ubuntu. * FOrce upgrade of DGL and matgl. * Make the MPResterBasic interface mirror the mp-api interface exactly. * pre-commit autoupdate (#4353) updates: - [github.com/astral-sh/ruff-pre-commit: v0.9.9 → v0.11.4](astral-sh/ruff-pre-commit@v0.9.9...v0.11.4) - [github.com/RobertCraigie/pyright-python: v1.1.396 → v1.1.398](RobertCraigie/pyright-python@v1.1.396...v1.1.398) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Replace `to_dict` with `as_dict` (#4352) * deprecate to_dict * deprecate JDFTX with 6 months grace period * update compatibility log * more hidden replacement * revert to to_dict for dataframe * deprecate to_xxx in abitimer * directly rename dev/helper functions * more as_xxx * fix typo * deprecate composition * use args and kwargs to pass thru, would loss autocomplete * more to_xxx * last batch as_xxx * as_table_str * update usage in tests * fix some leftover * add naming convention note in contributing md * update compatibility log * TO BE REVERTED: mark future warning as error * add dummy docstring * fix some * should be everything to fix * revert everything first * Revert "update compatibility log" This reverts commit 87ab51c. * deprecate to_dict * deprecate JDFTX with 6 months grace period * change other to dict in core * update compatibility log * better alias * migrate internal usage * regenerate docs * Fix DGL. * Fix order of install. * AseAtomsAdaptor.get_structure: Do no constrain constrain-free sites (#4355) * src/pymatgen/io/ase.py (AseAtomsAdaptor.get_structure): When no explicit constraint is given for a site in ASE Atoms object, use "T T T" selective dynamics (no constraint). The old code is plain wrong. * tests/io/test_ase.py (test_back_forth_constraints): New test. Fixes #4354. Thanks to @yaoyi92 for reporting! Co-authored-by: Andrew S. Rosen <[email protected]> * Update documentation. * Updated changelog. * Update docs * USe uv to publish. * fix import (#4357) * Remove target-version and rely on requires python instead. * `core.Lattice` cast `pbc` to `bool` (possibly `np.bool_`) (#4356) * cast to bool * update test * Clean up optional Ubuntu CI pipeline, skip `BoltzTraP2 ` install in pipeline for now (#4347) * try to limit bt2 version for now * combine optional dep for ubuntu and macos * still doesn't work, comment out bt2 for now * one liner * use realpath * less verbose make * remove debug output * disable gcc warning * fix merge * revert comment out bt2 * try bt2 build for windows * bt2 seems to build fine on win runner now * comment out BoltzTraP2 for now --------- Co-authored-by: Shyue Ping Ong <[email protected]> * Add .python-version * Seems like micromamba doesn't like python version. * Properly use dependency groups for uv. * See if adding a uv sync solvs the problem. * Add back ci category. * Upgrade matgl. Remove dependency on chgnet since this has now been moved to within matgl, which is easier. * Fix workflow for Ubuntu. * FOrce upgrade of DGL and matgl. * Make the MPResterBasic interface mirror the mp-api interface exactly. * Replace `to_dict` with `as_dict` (#4352) * deprecate to_dict * deprecate JDFTX with 6 months grace period * update compatibility log * more hidden replacement * revert to to_dict for dataframe * deprecate to_xxx in abitimer * directly rename dev/helper functions * more as_xxx * fix typo * deprecate composition * use args and kwargs to pass thru, would loss autocomplete * more to_xxx * last batch as_xxx * as_table_str * update usage in tests * fix some leftover * add naming convention note in contributing md * update compatibility log * TO BE REVERTED: mark future warning as error * add dummy docstring * fix some * should be everything to fix * revert everything first * Revert "update compatibility log" This reverts commit 87ab51c. * deprecate to_dict * deprecate JDFTX with 6 months grace period * change other to dict in core * update compatibility log * better alias * migrate internal usage * regenerate docs * Fix DGL. * Fix order of install. * Update documentation. * Update docs * Clean up optional Ubuntu CI pipeline, skip `BoltzTraP2 ` install in pipeline for now (#4347) * try to limit bt2 version for now * combine optional dep for ubuntu and macos * still doesn't work, comment out bt2 for now * one liner * use realpath * less verbose make * remove debug output * disable gcc warning * fix merge * revert comment out bt2 * try bt2 build for windows * bt2 seems to build fine on win runner now * comment out BoltzTraP2 for now --------- Co-authored-by: Shyue Ping Ong <[email protected]> * Add .python-version * Seems like micromamba doesn't like python version. * Properly use dependency groups for uv. * See if adding a uv sync solvs the problem. * Add back ci category. * Upgrade matgl. Remove dependency on chgnet since this has now been moved to within matgl, which is easier. * Fix workflow for Ubuntu. * FOrce upgrade of DGL and matgl. * Make the MPResterBasic interface mirror the mp-api interface exactly. * Replace `to_dict` with `as_dict` (#4352) * deprecate to_dict * deprecate JDFTX with 6 months grace period * update compatibility log * more hidden replacement * revert to to_dict for dataframe * deprecate to_xxx in abitimer * directly rename dev/helper functions * more as_xxx * fix typo * deprecate composition * use args and kwargs to pass thru, would loss autocomplete * more to_xxx * last batch as_xxx * as_table_str * update usage in tests * fix some leftover * add naming convention note in contributing md * update compatibility log * TO BE REVERTED: mark future warning as error * add dummy docstring * fix some * should be everything to fix * revert everything first * Revert "update compatibility log" This reverts commit 87ab51c. * deprecate to_dict * deprecate JDFTX with 6 months grace period * change other to dict in core * update compatibility log * better alias * migrate internal usage * regenerate docs * Fix DGL. * Fix order of install. * Update documentation. * Update docs * Remove target-version and rely on requires python instead. * Clean up optional Ubuntu CI pipeline, skip `BoltzTraP2 ` install in pipeline for now (#4347) * try to limit bt2 version for now * combine optional dep for ubuntu and macos * still doesn't work, comment out bt2 for now * one liner * use realpath * less verbose make * remove debug output * disable gcc warning * fix merge * revert comment out bt2 * try bt2 build for windows * bt2 seems to build fine on win runner now * comment out BoltzTraP2 for now --------- Co-authored-by: Shyue Ping Ong <[email protected]> * Beginning repairs from merge. Also changing _jof_atr_from_last_slice from list into a tuple as it looks like that was supposed to be done on this one * missed fix * No fail detection yet, but fixing the none_slice_on_error behavior for from_calc_dir * (UNTESTED) additional lines for dealing with velocities * Expanding _get_colon_val to return np.nan on "nan" values, expanding JOutStructure to hold data for MD simulations, adding a JDFTXInfile object to JDFTXOutfileSlice to parse dumped input commands, beginning replacement of jminsettings objects with attached JDFTXInfile object, fixing old "null" strings from out files when jdftx briefly dumped an unreadable string, partiall replacing "np.ndarray" with "np.typing.NDArray" to indicate array typing * Support and testing for Rhombohedral, Hexagonal, and Triclinic lattice options * Support for all lattice types. TODO - revise the `read` method for TagContainer so lattice TagContainer options don't have to be duplicated with/without the "modification" subtag * Fix for JDFTXStructure.__str__() method * incorrect indexing of velocity line * Changing `ion` to a single-option MultiformatTag * Adding options for single contraint and multiple hyperplanes * Three format options for 'ion' tag - no constraints, one constraint, or multiple hyperplanes. For second option, "constraint type" and "d0"-"d2" are required unnamed subtags. For final option, "HyperPlane" constraint is treated as its own named subtag (so no "constraint type" subtag, only a subtag name "HyperPlane" with a list of TagContainers containing "d0"-"d2" and "group" subtags) * Giving consistent number of decimals for "d0"-"d2" * Cleaning up commented out code * partially tested support for constraints in JDFTXStructure * docustrings and comments * Moving new JDFTXStructure site properties to regular attributes as all site_properties need to have homogenous shaping. Making typing of new attributes fairly undescriptive for now to pass pre-commit * Protecting possible inhomogenous values from site properties * roll back JDFTXStructure attributes to None if all entries are None * get_colon_val alias for partial merging to materialsproject * Fixing ordering orbitals in bandprojection parsing, adding alias to get_colon_val for partial merging to materialsproject * typing updates * adding tests for updated lattice and ion tag objects to jdftxinfile test module * Removing commented out out-dated test * Removing commented out code * Removing commented out code * Removing commented out code * updated test orbital ordering * Fixes for updated mypy typing criteria * Moved tests * Unused test * Removing commented out test --------- Co-authored-by: cote3804 <[email protected]> Co-authored-by: Cooper <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Jacob Clary <[email protected]> Co-authored-by: Shyue Ping Ong <[email protected]> Co-authored-by: Charles Cardot <[email protected]> Co-authored-by: Shyue Ping Ong <[email protected]> Co-authored-by: Haoyu (Daniel) YANG 杨浩宇 <[email protected]> Co-authored-by: Ihor Radchenko <[email protected]> Co-authored-by: Andrew S. Rosen <[email protected]>
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.
SymmOp, to closeImportError: cannot import name'SymmetryGroup'from partially initialized module'pymatgen.symmetry.groups'#4309Looks like the import of
SymmOpwas added in #4177, @tpurcell90 do we have a reason to importSymmOpat runtime?pymatgen/src/pymatgen/symmetry/groups.py
Lines 24 to 33 in 482ffd0