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.

Sign up for GitHub

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

Jump to bottom

Move test structures out of util directory #3831

Merged
merged 1 commit into from
May 15, 2024
Merged

Move test structures out of util directory #3831

merged 1 commit into from
May 15, 2024

Conversation

DanielYang59
Copy link
Contributor

@DanielYang59 DanielYang59 commented May 15, 2024 •
edited
Loading

Summary

  • Move test structures out of util directory

We might need a better name for the directory?

@janosh janosh added tests Issues with or changes to the pymatgen test suite housekeeping Moving around or cleaning up old code/files labels May 15, 2024
janosh
janosh approved these changes May 15, 2024
View reviewed changes
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 @DanielYang59! 👍 i don't recall why those structures weren't in the tests/files folder to begin with

@janosh janosh merged commit daa5c75 into materialsproject:master May 15, 2024
23 checks passed
@DanielYang59 DanielYang59 deleted the move-structs-outside-util branch May 15, 2024 11:53
@DanielYang59
Copy link
Contributor Author

Thanks for reviewing.

Meanwhile I would say uv install torch seems to have a higher and higher failure rate for some reason. We have always been seeing it to fail. Should we consider revert to slower but more reliable pip?

@janosh
Copy link
Member

janosh commented May 15, 2024 •
edited
Loading

we're already installing it with pip. maybe there's a flag in uv pip install to tell it to ignore packages which are already installed? just to confirm, currently uv tries to install pytorch again?

pymatgen/.github/workflows/test.yml

Line 73 in daa5c75

pip install torch

@DanielYang59
Copy link
Contributor Author

we're already installing it with pip.

Yes we are.

maybe there's a flag in uv pip install to tell it to ignore packages which are already installed? just to confirm, currently uv tries to install pytorch again?

I'm not aware of such arg, but I would do some research and let you know.

I don't know how uv/pip works under the hood, but to detect installed packages, uv seems to need to same search process just like install, and once this failed, the install stage would fail.

So you are right, we need to explicitly tell uv to ignore torch, we cannot let it detect itself

DanielYang59 added a commit to DanielYang59/pymatgen that referenced this pull request May 16, 2024
@DanielYang59 @janosh
relocate dunder methods to top for core.trajectory
1683c18 
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
@DanielYang59 @janosh
Improve type annotations for core.(trajectory/units) (#3832)
6a84d8a 
* 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 pushed a commit to esoteric-ephemera/pymatgen that referenced this pull request May 17, 2024
@DanielYang59 @janosh
move structures out of util dir (materialsproject#3831)
9e89ec5
@janosh janosh mentioned this pull request Jun 10, 2024
Closed
shyuep added a commit that referenced this pull request Jun 11, 2024
@shyuep
Revert "move structures out of util dir (#3831)"
5a020ee 
This reverts commit daa5c75.
@shyuep shyuep mentioned this pull request Jun 11, 2024
Merged
shyuep added a commit that referenced this pull request Jun 11, 2024
@shyuep
Revert "move structures out of util dir (#3831)" (#3875)
5144d8b 
This reverts commit daa5c75.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@janosh janosh janosh approved these changes

@shyuep shyuep Awaiting requested review from shyuep shyuep is a code owner

@mkhorton mkhorton Awaiting requested review from mkhorton mkhorton is a code owner

Assignees
No one assigned
Labels
housekeeping Moving around or cleaning up old code/files tests Issues with or changes to the pymatgen test suite
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DanielYang59 @janosh