From e840571de87e8f9fb381d10ef5d9d15abef770a5 Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Mon, 26 Aug 2024 14:24:03 +0800 Subject: [PATCH 01/25] remove package-data section --- pyproject.toml | 26 +------------------------- 1 file changed, 1 insertion(+), 25 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 8d302352ea0..e92fbe0cbab 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -124,36 +124,12 @@ feff_plot_dos = "pymatgen.cli.feff_plot_dos:main" get_environment = "pymatgen.cli.get_environment:main" [tool.setuptools] -include-package-data = false +include-package-data = true [tool.setuptools.packages.find] where = ["src"] include = ["pymatgen", "pymatgen.*"] -[tool.setuptools.package-data] -"pymatgen.analysis" = ["*.csv", "*.json", "*.yaml"] -"pymatgen.analysis.chemenv" = [ - "coordination_environments/coordination_geometries_files/*.json", - "coordination_environments/coordination_geometries_files/*.txt", - "coordination_environments/strategy_files/ImprovedConfidenceCutoffDefaultParameters.json", -] -"pymatgen.analysis.structure_prediction" = ["*.yaml", "data/*.json"] -"pymatgen.analysis.diffraction" = ["*.json"] -"pymatgen.analysis.magnetism" = ["default_magmoms.yaml"] -"pymatgen.analysis.solar" = ["am1.5G.dat"] -"pymatgen.entries" = ["*.json.gz", "*.yaml", "data/*.json"] -"pymatgen.core" = ["*.json"] -"pymatgen" = ["py.typed"] -"pymatgen.io.vasp" = ["*.json", "*.json.bz2", "*.json.gz", "*.yaml"] -"pymatgen.io.feff" = ["*.yaml"] -"pymatgen.io.cp2k" = ["*.yaml"] -"pymatgen.io.lobster" = ["lobster_basis/*.yaml"] -"pymatgen.command_line" = ["*"] -"pymatgen.util" = ["*.json", "structures/*.json"] -"pymatgen.vis" = ["*.yaml"] -"pymatgen.io.lammps" = ["CoeffsDataType.yaml", "templates/*.template"] -"pymatgen.symmetry" = ["*.json", "*.sqlite", "*.yaml"] - [tool.pdm.dev-dependencies] lint = ["mypy>=1.10.0", "pre-commit>=3.7.1", "ruff>=0.4.9"] test = ["pytest-cov>=5.0.0", "pytest-split>=0.9.0", "pytest>=8.2.2"] From 43fe42dbea8040127a3ac0b20e288dd13ca49c04 Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Mon, 26 Aug 2024 15:07:55 +0800 Subject: [PATCH 02/25] WIP: fix some reason files listed in exclude-package-data is still included in wheel --- pyproject.toml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index e92fbe0cbab..6fbbb184d07 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -126,6 +126,16 @@ get_environment = "pymatgen.cli.get_environment:main" [tool.setuptools] include-package-data = true +[tool.setuptools.exclude-package-data] +"pymatgen" = [ + "optimization/linear_assignment.c", + "optimization/linear_assignment.pyx", + "optimization/neighbors.c", + "optimization/neighbors.pyx", + "util/coord_cython.c", + "util/coord_cython.pyx", +] + [tool.setuptools.packages.find] where = ["src"] include = ["pymatgen", "pymatgen.*"] From 967952c8c5c3fd2c1613f6ff243fff59714f3a49 Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Sat, 7 Sep 2024 10:51:01 +0800 Subject: [PATCH 03/25] update omitted files for coverage --- pyproject.toml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 5db3c87730c..fb92fbb8673 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -249,12 +249,10 @@ addopts = "--durations=30 --quiet -r xXs --color=yes -p no:warnings --import-mod [tool.coverage.run] parallel = true omit = [ - "pymatgen/cli/feff_plot_cross_section.py", - "pymatgen/cli/feff_plot_dos.py", + "pymatgen/cli/feff_*.py", "pymatgen/cli/pmg_config.py", "pymatgen/cli/pmg_plot.py", "pymatgen/cli/pmg_potcar.py", - "pymatgen/cli/pmg_query.py", "pymatgen/dao.py", ] From c8a3a50e3e92a3a66bd782d75e3f53e0f7252e76 Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Sat, 7 Sep 2024 11:58:31 +0800 Subject: [PATCH 04/25] include everything for now --- pyproject.toml | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index fb92fbb8673..0fdc1905bd4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -133,16 +133,6 @@ get_environment = "pymatgen.cli.get_environment:main" [tool.setuptools] include-package-data = true -[tool.setuptools.exclude-package-data] -"pymatgen" = [ - "optimization/linear_assignment.c", - "optimization/linear_assignment.pyx", - "optimization/neighbors.c", - "optimization/neighbors.pyx", - "util/coord_cython.c", - "util/coord_cython.pyx", -] - [tool.setuptools.packages.find] where = ["src"] include = ["pymatgen", "pymatgen.*"] From 605df2ca192434bcaeb2c3064155c77d49bd304c Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Wed, 18 Sep 2024 22:27:08 +0800 Subject: [PATCH 05/25] experimented with setuptools-scm, but no luck to exclude file --- pyproject.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index b50c0871be1..ae8791a86ce 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -130,8 +130,8 @@ feff_plot_cross_section = "pymatgen.cli.feff_plot_cross_section:main" feff_plot_dos = "pymatgen.cli.feff_plot_dos:main" get_environment = "pymatgen.cli.get_environment:main" -[tool.setuptools] -include-package-data = true +[tool.setuptools.package-data] + "*" = ["*", ] # include everything under src/ dir as data files [tool.setuptools.packages.find] where = ["src"] From 47025a640128f605a74e5d478913274fdf285eca Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Wed, 18 Sep 2024 22:42:44 +0800 Subject: [PATCH 06/25] maybe give setuptools-scm a chance, need really careful check of package content --- MANIFEST.in | 11 +++++++++++ pyproject.toml | 4 +--- 2 files changed, 12 insertions(+), 3 deletions(-) create mode 100644 MANIFEST.in diff --git a/MANIFEST.in b/MANIFEST.in new file mode 100644 index 00000000000..0612c6ac7f1 --- /dev/null +++ b/MANIFEST.in @@ -0,0 +1,11 @@ +# Exclude entire directories +prune .github/ +prune dev_scripts/ +prune docs/ +prune examples/ +prune tests/ + +# Exclude individual files +exclude .* \ + ADMIN.md CONTRIBUTING.md SECURITY.md \ + CITATION.cff pdm.lock tasks.py diff --git a/pyproject.toml b/pyproject.toml index ae8791a86ce..d345588fae5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,6 +5,7 @@ requires = [ # https://numpy.org/devdocs/dev/depending_on_numpy.html#build-time-dependency "numpy>=2.1.0", "setuptools>=65.0.0", + "setuptools-scm>=8", # include all Git tracked files as package data file ] build-backend = "setuptools.build_meta" @@ -130,9 +131,6 @@ feff_plot_cross_section = "pymatgen.cli.feff_plot_cross_section:main" feff_plot_dos = "pymatgen.cli.feff_plot_dos:main" get_environment = "pymatgen.cli.get_environment:main" -[tool.setuptools.package-data] - "*" = ["*", ] # include everything under src/ dir as data files - [tool.setuptools.packages.find] where = ["src"] include = ["pymatgen", "pymatgen.*"] From 49905f2cdfa8b58b9cbfefae4d357dbc5de229a6 Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Thu, 19 Sep 2024 16:08:01 +0800 Subject: [PATCH 07/25] fix py.typed placement --- src/pymatgen/{ => alchemy}/py.typed | 0 src/pymatgen/analysis/py.typed | 0 src/pymatgen/apps/py.typed | 0 src/pymatgen/cli/py.typed | 0 src/pymatgen/command_line/py.typed | 0 src/pymatgen/core/py.typed | 0 src/pymatgen/electronic_structure/py.typed | 0 src/pymatgen/entries/py.typed | 0 src/pymatgen/ext/py.typed | 0 src/pymatgen/io/py.typed | 0 src/pymatgen/optimization/py.typed | 0 src/pymatgen/phonon/py.typed | 0 src/pymatgen/symmetry/py.typed | 0 src/pymatgen/transformations/py.typed | 0 src/pymatgen/util/py.typed | 0 src/pymatgen/vis/py.typed | 0 16 files changed, 0 insertions(+), 0 deletions(-) rename src/pymatgen/{ => alchemy}/py.typed (100%) create mode 100644 src/pymatgen/analysis/py.typed create mode 100644 src/pymatgen/apps/py.typed create mode 100644 src/pymatgen/cli/py.typed create mode 100644 src/pymatgen/command_line/py.typed create mode 100644 src/pymatgen/core/py.typed create mode 100644 src/pymatgen/electronic_structure/py.typed create mode 100644 src/pymatgen/entries/py.typed create mode 100644 src/pymatgen/ext/py.typed create mode 100644 src/pymatgen/io/py.typed create mode 100644 src/pymatgen/optimization/py.typed create mode 100644 src/pymatgen/phonon/py.typed create mode 100644 src/pymatgen/symmetry/py.typed create mode 100644 src/pymatgen/transformations/py.typed create mode 100644 src/pymatgen/util/py.typed create mode 100644 src/pymatgen/vis/py.typed diff --git a/src/pymatgen/py.typed b/src/pymatgen/alchemy/py.typed similarity index 100% rename from src/pymatgen/py.typed rename to src/pymatgen/alchemy/py.typed diff --git a/src/pymatgen/analysis/py.typed b/src/pymatgen/analysis/py.typed new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/pymatgen/apps/py.typed b/src/pymatgen/apps/py.typed new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/pymatgen/cli/py.typed b/src/pymatgen/cli/py.typed new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/pymatgen/command_line/py.typed b/src/pymatgen/command_line/py.typed new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/pymatgen/core/py.typed b/src/pymatgen/core/py.typed new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/pymatgen/electronic_structure/py.typed b/src/pymatgen/electronic_structure/py.typed new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/pymatgen/entries/py.typed b/src/pymatgen/entries/py.typed new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/pymatgen/ext/py.typed b/src/pymatgen/ext/py.typed new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/pymatgen/io/py.typed b/src/pymatgen/io/py.typed new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/pymatgen/optimization/py.typed b/src/pymatgen/optimization/py.typed new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/pymatgen/phonon/py.typed b/src/pymatgen/phonon/py.typed new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/pymatgen/symmetry/py.typed b/src/pymatgen/symmetry/py.typed new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/pymatgen/transformations/py.typed b/src/pymatgen/transformations/py.typed new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/pymatgen/util/py.typed b/src/pymatgen/util/py.typed new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/pymatgen/vis/py.typed b/src/pymatgen/vis/py.typed new file mode 100644 index 00000000000..e69de29bb2d From 8f1bcc4a72dd1db903434d29cd72fa2584b6afba Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Thu, 14 Nov 2024 19:58:03 +0800 Subject: [PATCH 08/25] encourage google style docstring --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 722d157a9bf..d07cacfc25b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -87,6 +87,6 @@ Given that `pymatgen` is intended to be a long-term code base, we adopt very str ``` 1. **Python 3**. We only support Python 3.10+. -1. **Documentation** is required for all modules, classes and methods. In particular, the method doc strings should make clear the arguments expected and the return values. For complex algorithms (e.g., an Ewald summation), a summary of the algorithm should be provided and preferably with a link to a publication outlining the method in detail. +1. **Documentation** is required for all modules, classes and methods. We prefer [Google Style Docstrings](https://www.sphinx-doc.org/en/master/usage/extensions/example_google.html). In particular, the method doc strings should make clear the arguments expected and the return values. For complex algorithms (e.g., an Ewald summation), a summary of the algorithm should be provided and preferably with a link to a publication outlining the method in detail. For the above, if in doubt, please refer to the core classes in `pymatgen` for examples of what is expected. From b4388a86936f0754a2c1cd4167f205d6f4f37bcf Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Thu, 14 Nov 2024 19:59:31 +0800 Subject: [PATCH 09/25] fix internal index --- CONTRIBUTING.md | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d07cacfc25b..57e07e92191 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -10,11 +10,11 @@ For developers interested in expanding `pymatgen` for their own purposes, we rec 1. Create a free GitHub account (if you don't already have one) and perform the necessary setup (e.g., install SSH keys etc.). -1. Fork the `pymatgen` GitHub repo, i.e., go to the main [`pymatgen` GitHub repo](https://github.com/materialsproject/pymatgen) and click fork to create a copy of the `pymatgen` code base on your own GitHub account. +2. Fork the `pymatgen` GitHub repo, i.e., go to the main [`pymatgen` GitHub repo](https://github.com/materialsproject/pymatgen) and click fork to create a copy of the `pymatgen` code base on your own GitHub account. -1. Install `git` on your local machine (if you don't already have it). +3. Install `git` on your local machine (if you don't already have it). -1. Clone *your forked repo* to your local machine. You will work mostly with your local repo and only publish changes when they are ready to be merged: +4. Clone *your forked repo* to your local machine. You will work mostly with your local repo and only publish changes when they are ready to be merged: ```sh git clone https://github.com//pymatgen @@ -22,13 +22,13 @@ For developers interested in expanding `pymatgen` for their own purposes, we rec Note that the entire Github repo is fairly large because of the presence of test files, but these are necessary for rigorous testing. -1. Make a new branch for your contributions +5. Make a new branch for your contributions ```sh git checkout -b my-new-fix-or-feature # should be run from up-to-date master ``` -1. Code (see [Coding Guidelines](#coding-guidelines)). Commit early and commit often. Keep your code up to date. You need to add the main repository to the list of your remotes. +6. Code (see [Coding Guidelines](#coding-guidelines)). Commit early and commit often. Keep your code up to date. You need to add the main repository to the list of your remotes. ```sh git remote add upstream https://github.com/materialsproject/pymatgen @@ -48,19 +48,19 @@ For developers interested in expanding `pymatgen` for their own purposes, we rec Remember, pull is a combination of the commands fetch and merge, so there may be merge conflicts to be manually resolved. -1. Publish your contributions. Assuming that you now have a couple of commits that you would like to contribute to the main repository. Please follow the following steps: +7. Publish your contributions. Assuming that you now have a couple of commits that you would like to contribute to the main repository. Please follow the following steps: 1. If your change is based on a relatively old state of the main repository, then you should probably bring your repository up-to-date first to see if the change is not creating any merge conflicts. - 1. Check that everything compiles cleanly and passes all tests. The `pymatgen` repo comes with a complete set of tests for all modules. If you have written new modules or methods, you must write tests for the new code as well (see [Coding Guidelines](#coding-guidelines)). Install and run `pytest` in your local repo directory and fix all errors before continuing further. + 2. Check that everything compiles cleanly and passes all tests. The `pymatgen` repo comes with a complete set of tests for all modules. If you have written new modules or methods, you must write tests for the new code as well (see [Coding Guidelines](#coding-guidelines)). Install and run `pytest` in your local repo directory and fix all errors before continuing further. - 1. If everything is ok, publish the commits to your GitHub repository. + 3. If everything is ok, publish the commits to your GitHub repository. ```sh git push origin master ``` -1. Now that your commit is published, it doesn't mean that it has already been merged into the main repository. You should issue a merge request to `pymatgen` maintainers. They will pull your commits and run their own tests before releasing. +8. Now that your commit is published, it doesn't mean that it has already been merged into the main repository. You should issue a merge request to `pymatgen` maintainers. They will pull your commits and run their own tests before releasing. "Work-in-progress" pull requests are encouraged, especially if this is your first time contributing to `pymatgen`, and the maintainers will be happy to help or provide code review as necessary. Put "\[WIP\]" in the title of your pull request to indicate it's not ready to be merged. @@ -77,7 +77,7 @@ Given that `pymatgen` is intended to be a long-term code base, we adopt very str PMG_TEST_FILES_DIR=$(pwd)/tests/files pytest tests # run the test suite providing the path for the datafiles ``` -1. **Python PEP 8** [code style](https://python.org/dev/peps/pep-0008). We allow a few exceptions when they are well-justified (e.g., Element's atomic number is given a variable name of capital Z, in line with accepted scientific convention), but generally, PEP 8 must be observed. Code style will be automatically checked for all PRs and must pass before any PR is merged. To aid you, you can install and run the same set of formatters and linters that will run in CI using +2. **Python PEP 8** [code style](https://python.org/dev/peps/pep-0008). We allow a few exceptions when they are well-justified (e.g., Element's atomic number is given a variable name of capital Z, in line with accepted scientific convention), but generally, PEP 8 must be observed. Code style will be automatically checked for all PRs and must pass before any PR is merged. To aid you, you can install and run the same set of formatters and linters that will run in CI using ```sh pre-commit install # ensures linters are run prior to all future commits @@ -86,7 +86,7 @@ Given that `pymatgen` is intended to be a long-term code base, we adopt very str pre-commit run --all-files # ensure your entire codebase passes linters ``` -1. **Python 3**. We only support Python 3.10+. -1. **Documentation** is required for all modules, classes and methods. We prefer [Google Style Docstrings](https://www.sphinx-doc.org/en/master/usage/extensions/example_google.html). In particular, the method doc strings should make clear the arguments expected and the return values. For complex algorithms (e.g., an Ewald summation), a summary of the algorithm should be provided and preferably with a link to a publication outlining the method in detail. +3. **Python 3**. We only support Python 3.10+. +4. **Documentation** is required for all modules, classes and methods. We prefer [Google Style Docstrings](https://www.sphinx-doc.org/en/master/usage/extensions/example_google.html). In particular, the method doc strings should make clear the arguments expected and the return values. For complex algorithms (e.g., an Ewald summation), a summary of the algorithm should be provided and preferably with a link to a publication outlining the method in detail. For the above, if in doubt, please refer to the core classes in `pymatgen` for examples of what is expected. From 2c50c00b6556d3c373401893c768628369c66ee5 Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Thu, 14 Nov 2024 20:07:59 +0800 Subject: [PATCH 10/25] TEST, test install from sdist --- .github/workflows/test.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 852fa8719f0..56731983ab4 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -86,10 +86,11 @@ jobs: pip install torch==2.2.1 # Install from wheels to test the content + # DEBUG: test sdist, revert after test passed uv pip install build - python -m build --wheel + python -m build --sdist - uv pip install dist/*.whl + uv pip install dist/*.tar.gz uv pip install pymatgen[${{ matrix.config.extras }}] --resolution=${{ matrix.config.resolution }} - name: Install optional Ubuntu dependencies From b1ef965b63c6a24c3cb0d18ed543d1d3565b764f Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Thu, 14 Nov 2024 20:15:54 +0800 Subject: [PATCH 11/25] Revert "TEST, test install from sdist" This reverts commit 2c50c00b6556d3c373401893c768628369c66ee5. --- .github/workflows/test.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 56731983ab4..852fa8719f0 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -86,11 +86,10 @@ jobs: pip install torch==2.2.1 # Install from wheels to test the content - # DEBUG: test sdist, revert after test passed uv pip install build - python -m build --sdist + python -m build --wheel - uv pip install dist/*.tar.gz + uv pip install dist/*.whl uv pip install pymatgen[${{ matrix.config.extras }}] --resolution=${{ matrix.config.resolution }} - name: Install optional Ubuntu dependencies From 8482fff474930fae886fbe669dc4429da536838c Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Fri, 15 Nov 2024 12:42:54 +0800 Subject: [PATCH 12/25] revert contributing change --- CONTRIBUTING.md | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 57e07e92191..722d157a9bf 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -10,11 +10,11 @@ For developers interested in expanding `pymatgen` for their own purposes, we rec 1. Create a free GitHub account (if you don't already have one) and perform the necessary setup (e.g., install SSH keys etc.). -2. Fork the `pymatgen` GitHub repo, i.e., go to the main [`pymatgen` GitHub repo](https://github.com/materialsproject/pymatgen) and click fork to create a copy of the `pymatgen` code base on your own GitHub account. +1. Fork the `pymatgen` GitHub repo, i.e., go to the main [`pymatgen` GitHub repo](https://github.com/materialsproject/pymatgen) and click fork to create a copy of the `pymatgen` code base on your own GitHub account. -3. Install `git` on your local machine (if you don't already have it). +1. Install `git` on your local machine (if you don't already have it). -4. Clone *your forked repo* to your local machine. You will work mostly with your local repo and only publish changes when they are ready to be merged: +1. Clone *your forked repo* to your local machine. You will work mostly with your local repo and only publish changes when they are ready to be merged: ```sh git clone https://github.com//pymatgen @@ -22,13 +22,13 @@ For developers interested in expanding `pymatgen` for their own purposes, we rec Note that the entire Github repo is fairly large because of the presence of test files, but these are necessary for rigorous testing. -5. Make a new branch for your contributions +1. Make a new branch for your contributions ```sh git checkout -b my-new-fix-or-feature # should be run from up-to-date master ``` -6. Code (see [Coding Guidelines](#coding-guidelines)). Commit early and commit often. Keep your code up to date. You need to add the main repository to the list of your remotes. +1. Code (see [Coding Guidelines](#coding-guidelines)). Commit early and commit often. Keep your code up to date. You need to add the main repository to the list of your remotes. ```sh git remote add upstream https://github.com/materialsproject/pymatgen @@ -48,19 +48,19 @@ For developers interested in expanding `pymatgen` for their own purposes, we rec Remember, pull is a combination of the commands fetch and merge, so there may be merge conflicts to be manually resolved. -7. Publish your contributions. Assuming that you now have a couple of commits that you would like to contribute to the main repository. Please follow the following steps: +1. Publish your contributions. Assuming that you now have a couple of commits that you would like to contribute to the main repository. Please follow the following steps: 1. If your change is based on a relatively old state of the main repository, then you should probably bring your repository up-to-date first to see if the change is not creating any merge conflicts. - 2. Check that everything compiles cleanly and passes all tests. The `pymatgen` repo comes with a complete set of tests for all modules. If you have written new modules or methods, you must write tests for the new code as well (see [Coding Guidelines](#coding-guidelines)). Install and run `pytest` in your local repo directory and fix all errors before continuing further. + 1. Check that everything compiles cleanly and passes all tests. The `pymatgen` repo comes with a complete set of tests for all modules. If you have written new modules or methods, you must write tests for the new code as well (see [Coding Guidelines](#coding-guidelines)). Install and run `pytest` in your local repo directory and fix all errors before continuing further. - 3. If everything is ok, publish the commits to your GitHub repository. + 1. If everything is ok, publish the commits to your GitHub repository. ```sh git push origin master ``` -8. Now that your commit is published, it doesn't mean that it has already been merged into the main repository. You should issue a merge request to `pymatgen` maintainers. They will pull your commits and run their own tests before releasing. +1. Now that your commit is published, it doesn't mean that it has already been merged into the main repository. You should issue a merge request to `pymatgen` maintainers. They will pull your commits and run their own tests before releasing. "Work-in-progress" pull requests are encouraged, especially if this is your first time contributing to `pymatgen`, and the maintainers will be happy to help or provide code review as necessary. Put "\[WIP\]" in the title of your pull request to indicate it's not ready to be merged. @@ -77,7 +77,7 @@ Given that `pymatgen` is intended to be a long-term code base, we adopt very str PMG_TEST_FILES_DIR=$(pwd)/tests/files pytest tests # run the test suite providing the path for the datafiles ``` -2. **Python PEP 8** [code style](https://python.org/dev/peps/pep-0008). We allow a few exceptions when they are well-justified (e.g., Element's atomic number is given a variable name of capital Z, in line with accepted scientific convention), but generally, PEP 8 must be observed. Code style will be automatically checked for all PRs and must pass before any PR is merged. To aid you, you can install and run the same set of formatters and linters that will run in CI using +1. **Python PEP 8** [code style](https://python.org/dev/peps/pep-0008). We allow a few exceptions when they are well-justified (e.g., Element's atomic number is given a variable name of capital Z, in line with accepted scientific convention), but generally, PEP 8 must be observed. Code style will be automatically checked for all PRs and must pass before any PR is merged. To aid you, you can install and run the same set of formatters and linters that will run in CI using ```sh pre-commit install # ensures linters are run prior to all future commits @@ -86,7 +86,7 @@ Given that `pymatgen` is intended to be a long-term code base, we adopt very str pre-commit run --all-files # ensure your entire codebase passes linters ``` -3. **Python 3**. We only support Python 3.10+. -4. **Documentation** is required for all modules, classes and methods. We prefer [Google Style Docstrings](https://www.sphinx-doc.org/en/master/usage/extensions/example_google.html). In particular, the method doc strings should make clear the arguments expected and the return values. For complex algorithms (e.g., an Ewald summation), a summary of the algorithm should be provided and preferably with a link to a publication outlining the method in detail. +1. **Python 3**. We only support Python 3.10+. +1. **Documentation** is required for all modules, classes and methods. In particular, the method doc strings should make clear the arguments expected and the return values. For complex algorithms (e.g., an Ewald summation), a summary of the algorithm should be provided and preferably with a link to a publication outlining the method in detail. For the above, if in doubt, please refer to the core classes in `pymatgen` for examples of what is expected. From 3a1877b9b2b59fe906c9f672f0ac08afd7eb9e18 Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Fri, 15 Nov 2024 12:45:15 +0800 Subject: [PATCH 13/25] change default to False --- src/pymatgen/core/periodic_table.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pymatgen/core/periodic_table.py b/src/pymatgen/core/periodic_table.py index e9e91a9150f..5af0b05b0a0 100644 --- a/src/pymatgen/core/periodic_table.py +++ b/src/pymatgen/core/periodic_table.py @@ -1640,7 +1640,7 @@ def get_el_sp(obj: int | SpeciesLike) -> Element | Species | DummySpecies: """ # If obj is already an Element or Species, return as is if isinstance(obj, Element | Species | DummySpecies): - if getattr(obj, "_is_named_isotope", None): + if getattr(obj, "_is_named_isotope", False): return Element(obj.name) if isinstance(obj, Element) else Species(str(obj)) return obj From 04b8f3b34425a39ddaea9b23473f34b3e0cfac5c Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Fri, 15 Nov 2024 12:47:06 +0800 Subject: [PATCH 14/25] Revert "change default to False" This reverts commit 3a1877b9b2b59fe906c9f672f0ac08afd7eb9e18. --- src/pymatgen/core/periodic_table.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pymatgen/core/periodic_table.py b/src/pymatgen/core/periodic_table.py index 5af0b05b0a0..e9e91a9150f 100644 --- a/src/pymatgen/core/periodic_table.py +++ b/src/pymatgen/core/periodic_table.py @@ -1640,7 +1640,7 @@ def get_el_sp(obj: int | SpeciesLike) -> Element | Species | DummySpecies: """ # If obj is already an Element or Species, return as is if isinstance(obj, Element | Species | DummySpecies): - if getattr(obj, "_is_named_isotope", False): + if getattr(obj, "_is_named_isotope", None): return Element(obj.name) if isinstance(obj, Element) else Species(str(obj)) return obj From 7a2db541aeb1224519ad6c29b2a011f6b6741aae Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Mon, 25 Nov 2024 15:53:33 +0800 Subject: [PATCH 15/25] unskip test_pkg --- tests/test_pkg.py | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/tests/test_pkg.py b/tests/test_pkg.py index c47516a5796..9dab0cbf47b 100644 --- a/tests/test_pkg.py +++ b/tests/test_pkg.py @@ -3,33 +3,22 @@ import os from glob import glob -import pytest - SRC_TXT_PATH = "src/pymatgen.egg-info/SOURCES.txt" -@pytest.mark.skipif( - not os.path.isfile(SRC_TXT_PATH), - reason=f"{SRC_TXT_PATH=} not found. Run `pip install .` to create", -) def test_egg_sources_txt_is_complete(): - """Check that all source and data files in pymatgen/ are listed in pymatgen.egg-info/SOURCES.txt.""" + """Check that all source and data files in src/pymatgen/ are listed in pymatgen.egg-info/SOURCES.txt.""" with open(SRC_TXT_PATH, encoding="utf-8") as file: sources = file.read() - # check that all files listed in SOURCES.txt exist + # Check that all files listed in SOURCES.txt exist for src_file in sources.splitlines(): assert os.path.isfile(src_file), f"{src_file!r} does not exist!" - # check that all files in pymatgen/ are listed in SOURCES.txt - for ext in ("py", "json*", "yaml", "csv"): + # Check that all files in src/pymatgen/ are listed in SOURCES.txt + for ext in ("py", "json", "json.*", "yaml", "csv"): for filepath in glob(f"pymatgen/**/*.{ext}", recursive=True): unix_path = filepath.replace("\\", "/") - if unix_path.endswith("dao.py"): - continue if unix_path not in sources: - raise ValueError( - f"{unix_path} not found in {SRC_TXT_PATH}. check setup.py package_data for " - "outdated inclusion rules." - ) + raise ValueError(f"{unix_path} not found in {SRC_TXT_PATH}, check package data config") From fb84435afdcd5c905712a219fa7f269438946ccd Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Mon, 25 Nov 2024 16:40:23 +0800 Subject: [PATCH 16/25] refactor test_pkg --- tests/test_pkg.py | 53 ++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 8 deletions(-) diff --git a/tests/test_pkg.py b/tests/test_pkg.py index 9dab0cbf47b..59530406255 100644 --- a/tests/test_pkg.py +++ b/tests/test_pkg.py @@ -1,24 +1,61 @@ +"""Check the content of source code, source distribution and binary distribution.""" + from __future__ import annotations import os +import subprocess from glob import glob +from pathlib import Path + +import pytest + +SRC_DIR = Path(__file__).parent.parent + + +class TestCheckDistribution: + def test_source_code(self): + """Directly check the source code in the working directory.""" + src_text_path = SRC_DIR / "src/pymatgen.egg-info/SOURCES.txt" + + check_src_txt_is_complete(SRC_DIR, src_text_path) -SRC_TXT_PATH = "src/pymatgen.egg-info/SOURCES.txt" + @pytest.mark.skip(reason="WIP") # TODO: parameterize for bdist/sdist? + def test_source_distribution(self): + """Build the source distribution and verify its contents.""" + # Build the source distribution in ScratchDir + subprocess.run(["python", "-m", "build", "--sdist"], check=True) + # Decompress -def test_egg_sources_txt_is_complete(): - """Check that all source and data files in src/pymatgen/ are listed in pymatgen.egg-info/SOURCES.txt.""" + # Verify the distribution contents + check_src_txt_is_complete(sdist_path, SRC_TXT_PATH) - with open(SRC_TXT_PATH, encoding="utf-8") as file: + @pytest.mark.skip(reason="WIP") + def test_binary_distribution(self): + """Build the binary distribution (wheels) and verify its contents.""" + + +def check_src_txt_is_complete(src_dir: str, src_txt_path: str) -> None: + """Check that all source code and data files are listed in given SOURCES.txt. + + Args: + src_dir (str): Path to the source code directory. + src_txt_path (str): Path to the "SOURCES.txt" file. + """ + assert os.path.isdir(src_dir) + assert os.path.isfile(src_txt_path) + + with open(src_txt_path, encoding="utf-8") as file: sources = file.read() - # Check that all files listed in SOURCES.txt exist + # Check that all files listed in "SOURCES.txt" exist for src_file in sources.splitlines(): assert os.path.isfile(src_file), f"{src_file!r} does not exist!" # Check that all files in src/pymatgen/ are listed in SOURCES.txt for ext in ("py", "json", "json.*", "yaml", "csv"): - for filepath in glob(f"pymatgen/**/*.{ext}", recursive=True): - unix_path = filepath.replace("\\", "/") + for filepath in glob(f"{src_dir}/src/pymatgen/**/*.{ext}", recursive=True): + unix_path = os.path.relpath(filepath.replace("\\", "/"), start=src_dir) + if unix_path not in sources: - raise ValueError(f"{unix_path} not found in {SRC_TXT_PATH}, check package data config") + raise ValueError(f"{unix_path} not found in {src_txt_path}, check package data config") From 8ed49b2fa15f48dfaa428fce09d6088c6995666c Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Mon, 25 Nov 2024 18:50:12 +0800 Subject: [PATCH 17/25] add test_source_distribution --- tests/test_pkg.py | 47 +++++++++++++++++++++++++++++++++-------------- 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/tests/test_pkg.py b/tests/test_pkg.py index 59530406255..bf59a91ee1b 100644 --- a/tests/test_pkg.py +++ b/tests/test_pkg.py @@ -4,10 +4,16 @@ import os import subprocess +import tarfile from glob import glob from pathlib import Path +from typing import TYPE_CHECKING import pytest +from monty.tempfile import ScratchDir + +if TYPE_CHECKING: + from pymatgen.util.typing import PathLike SRC_DIR = Path(__file__).parent.parent @@ -15,42 +21,55 @@ class TestCheckDistribution: def test_source_code(self): """Directly check the source code in the working directory.""" - src_text_path = SRC_DIR / "src/pymatgen.egg-info/SOURCES.txt" + src_txt_path = SRC_DIR / "src/pymatgen.egg-info/SOURCES.txt" - check_src_txt_is_complete(SRC_DIR, src_text_path) + _check_src_txt_is_complete(SRC_DIR, src_txt_path) - @pytest.mark.skip(reason="WIP") # TODO: parameterize for bdist/sdist? def test_source_distribution(self): """Build the source distribution and verify its contents.""" - # Build the source distribution in ScratchDir - subprocess.run(["python", "-m", "build", "--sdist"], check=True) - # Decompress + with ScratchDir("."): + # Build the source distribution + subprocess.run(["python", "-m", "build", "--sdist", SRC_DIR, "--outdir", ".", "-C--quiet"], check=True) + + # Decompress sdist + sdist_file = next(Path(".").glob("*.tar.gz")) + sdist_dir = sdist_file.name.removesuffix(".tar.gz") + with tarfile.open(sdist_file, "r:gz") as tar: + # TODO: remove attr check after only 3.12+ + if hasattr(tarfile, "data_filter"): + tar.extractall("", filter="data") + else: + tar.extractall("") # noqa: S202 - # Verify the distribution contents - check_src_txt_is_complete(sdist_path, SRC_TXT_PATH) + # Verify source distribution contents + src_txt_path = f"{sdist_dir}/src/pymatgen.egg-info/SOURCES.txt" + _check_src_txt_is_complete(src_dir=sdist_dir, src_txt_path=src_txt_path) @pytest.mark.skip(reason="WIP") def test_binary_distribution(self): """Build the binary distribution (wheels) and verify its contents.""" -def check_src_txt_is_complete(src_dir: str, src_txt_path: str) -> None: +def _check_src_txt_is_complete(src_dir: PathLike, src_txt_path: PathLike) -> None: """Check that all source code and data files are listed in given SOURCES.txt. Args: - src_dir (str): Path to the source code directory. - src_txt_path (str): Path to the "SOURCES.txt" file. + src_dir (PathLike): Path to the source code directory. + src_txt_path (PathLike): Path to the "SOURCES.txt" file. """ - assert os.path.isdir(src_dir) - assert os.path.isfile(src_txt_path) + src_dir = Path(src_dir) + src_txt_path = Path(src_txt_path) + + assert src_dir.is_dir(), f"{src_dir} is not a directory" + assert src_txt_path.is_file(), f"{src_txt_path} doesn't exist" with open(src_txt_path, encoding="utf-8") as file: sources = file.read() # Check that all files listed in "SOURCES.txt" exist for src_file in sources.splitlines(): - assert os.path.isfile(src_file), f"{src_file!r} does not exist!" + assert os.path.isfile(src_dir / src_file), f"{src_file!r} does not exist!" # Check that all files in src/pymatgen/ are listed in SOURCES.txt for ext in ("py", "json", "json.*", "yaml", "csv"): From de0da3fa7179d6412e18c83359be0918bdcf9b1c Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Mon, 25 Nov 2024 18:56:59 +0800 Subject: [PATCH 18/25] remove bdist build --- tests/test_pkg.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/test_pkg.py b/tests/test_pkg.py index bf59a91ee1b..cb686918828 100644 --- a/tests/test_pkg.py +++ b/tests/test_pkg.py @@ -1,4 +1,7 @@ -"""Check the content of source code, source distribution and binary distribution.""" +"""Check the content of source code and source distribution. + +Binary distribution (wheel) is checked in test workflow. +""" from __future__ import annotations @@ -9,7 +12,6 @@ from pathlib import Path from typing import TYPE_CHECKING -import pytest from monty.tempfile import ScratchDir if TYPE_CHECKING: @@ -46,10 +48,6 @@ def test_source_distribution(self): src_txt_path = f"{sdist_dir}/src/pymatgen.egg-info/SOURCES.txt" _check_src_txt_is_complete(src_dir=sdist_dir, src_txt_path=src_txt_path) - @pytest.mark.skip(reason="WIP") - def test_binary_distribution(self): - """Build the binary distribution (wheels) and verify its contents.""" - def _check_src_txt_is_complete(src_dir: PathLike, src_txt_path: PathLike) -> None: """Check that all source code and data files are listed in given SOURCES.txt. From 7b48912f6594b5408e1d12fc99f0dbc2c60fe90d Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Mon, 25 Nov 2024 22:32:12 +0800 Subject: [PATCH 19/25] fix failure in Windows and avoid hard-coded slash --- tests/test_pkg.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_pkg.py b/tests/test_pkg.py index cb686918828..2f29afeb5cb 100644 --- a/tests/test_pkg.py +++ b/tests/test_pkg.py @@ -5,7 +5,6 @@ from __future__ import annotations -import os import subprocess import tarfile from glob import glob @@ -32,6 +31,7 @@ def test_source_distribution(self): with ScratchDir("."): # Build the source distribution + subprocess.run(["python", "-m", "pip", "install", "--upgrade", "build"], check=True) subprocess.run(["python", "-m", "build", "--sdist", SRC_DIR, "--outdir", ".", "-C--quiet"], check=True) # Decompress sdist @@ -67,12 +67,12 @@ def _check_src_txt_is_complete(src_dir: PathLike, src_txt_path: PathLike) -> Non # Check that all files listed in "SOURCES.txt" exist for src_file in sources.splitlines(): - assert os.path.isfile(src_dir / src_file), f"{src_file!r} does not exist!" + assert (src_dir / src_file).is_file(), f"{src_file!r} does not exist!" # Check that all files in src/pymatgen/ are listed in SOURCES.txt for ext in ("py", "json", "json.*", "yaml", "csv"): for filepath in glob(f"{src_dir}/src/pymatgen/**/*.{ext}", recursive=True): - unix_path = os.path.relpath(filepath.replace("\\", "/"), start=src_dir) + unix_path = Path(filepath).relative_to(src_dir).as_posix() if unix_path not in sources: raise ValueError(f"{unix_path} not found in {src_txt_path}, check package data config") From 3b3b040c0a195cfb888abe627bcc10955791226f Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Mon, 25 Nov 2024 22:38:38 +0800 Subject: [PATCH 20/25] rename src_dir to avoid confusion --- tests/test_pkg.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/test_pkg.py b/tests/test_pkg.py index 2f29afeb5cb..fa8d6772543 100644 --- a/tests/test_pkg.py +++ b/tests/test_pkg.py @@ -16,15 +16,15 @@ if TYPE_CHECKING: from pymatgen.util.typing import PathLike -SRC_DIR = Path(__file__).parent.parent +PROJECT_ROOT = Path(__file__).parent.parent class TestCheckDistribution: def test_source_code(self): """Directly check the source code in the working directory.""" - src_txt_path = SRC_DIR / "src/pymatgen.egg-info/SOURCES.txt" + src_txt_path = PROJECT_ROOT / "src/pymatgen.egg-info/SOURCES.txt" - _check_src_txt_is_complete(SRC_DIR, src_txt_path) + _check_src_txt_is_complete(PROJECT_ROOT, src_txt_path) def test_source_distribution(self): """Build the source distribution and verify its contents.""" @@ -32,7 +32,7 @@ def test_source_distribution(self): with ScratchDir("."): # Build the source distribution subprocess.run(["python", "-m", "pip", "install", "--upgrade", "build"], check=True) - subprocess.run(["python", "-m", "build", "--sdist", SRC_DIR, "--outdir", ".", "-C--quiet"], check=True) + subprocess.run(["python", "-m", "build", "--sdist", PROJECT_ROOT, "--outdir", ".", "-C--quiet"], check=True) # Decompress sdist sdist_file = next(Path(".").glob("*.tar.gz")) @@ -46,20 +46,20 @@ def test_source_distribution(self): # Verify source distribution contents src_txt_path = f"{sdist_dir}/src/pymatgen.egg-info/SOURCES.txt" - _check_src_txt_is_complete(src_dir=sdist_dir, src_txt_path=src_txt_path) + _check_src_txt_is_complete(project_root=sdist_dir, src_txt_path=src_txt_path) -def _check_src_txt_is_complete(src_dir: PathLike, src_txt_path: PathLike) -> None: +def _check_src_txt_is_complete(project_root: PathLike, src_txt_path: PathLike) -> None: """Check that all source code and data files are listed in given SOURCES.txt. Args: - src_dir (PathLike): Path to the source code directory. + project_root (PathLike): Path to the directory containing "src/pymatgen/". src_txt_path (PathLike): Path to the "SOURCES.txt" file. """ - src_dir = Path(src_dir) + project_root = Path(project_root) src_txt_path = Path(src_txt_path) - assert src_dir.is_dir(), f"{src_dir} is not a directory" + assert project_root.is_dir(), f"{project_root} is not a directory" assert src_txt_path.is_file(), f"{src_txt_path} doesn't exist" with open(src_txt_path, encoding="utf-8") as file: @@ -67,12 +67,12 @@ def _check_src_txt_is_complete(src_dir: PathLike, src_txt_path: PathLike) -> Non # Check that all files listed in "SOURCES.txt" exist for src_file in sources.splitlines(): - assert (src_dir / src_file).is_file(), f"{src_file!r} does not exist!" + assert (project_root / src_file).is_file(), f"{src_file!r} does not exist!" # Check that all files in src/pymatgen/ are listed in SOURCES.txt for ext in ("py", "json", "json.*", "yaml", "csv"): - for filepath in glob(f"{src_dir}/src/pymatgen/**/*.{ext}", recursive=True): - unix_path = Path(filepath).relative_to(src_dir).as_posix() + for filepath in glob(f"{project_root}/src/pymatgen/**/*.{ext}", recursive=True): + unix_path = Path(filepath).relative_to(project_root).as_posix() if unix_path not in sources: raise ValueError(f"{unix_path} not found in {src_txt_path}, check package data config") From 9909527a72f0f8ce0a1cb158f9a89ea7449a3fbf Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Mon, 25 Nov 2024 22:41:03 +0800 Subject: [PATCH 21/25] simplify file reading for Path obj --- tests/test_pkg.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_pkg.py b/tests/test_pkg.py index fa8d6772543..cc7f546ce1c 100644 --- a/tests/test_pkg.py +++ b/tests/test_pkg.py @@ -62,8 +62,7 @@ def _check_src_txt_is_complete(project_root: PathLike, src_txt_path: PathLike) - assert project_root.is_dir(), f"{project_root} is not a directory" assert src_txt_path.is_file(), f"{src_txt_path} doesn't exist" - with open(src_txt_path, encoding="utf-8") as file: - sources = file.read() + sources = src_txt_path.read_text(encoding="utf-8") # Check that all files listed in "SOURCES.txt" exist for src_file in sources.splitlines(): From 4bdb549febef34c6681b00b7b02968b079bae270 Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Mon, 25 Nov 2024 23:00:29 +0800 Subject: [PATCH 22/25] avoid checking SOURCES.txt in sdist --- tests/test_pkg.py | 38 +++++++++++--------------------------- 1 file changed, 11 insertions(+), 27 deletions(-) diff --git a/tests/test_pkg.py b/tests/test_pkg.py index cc7f546ce1c..26b5c8f8b8a 100644 --- a/tests/test_pkg.py +++ b/tests/test_pkg.py @@ -1,4 +1,4 @@ -"""Check the content of source code and source distribution. +"""Check the content of source code and test build source distribution. Binary distribution (wheel) is checked in test workflow. """ @@ -6,7 +6,6 @@ from __future__ import annotations import subprocess -import tarfile from glob import glob from pathlib import Path from typing import TYPE_CHECKING @@ -19,34 +18,19 @@ PROJECT_ROOT = Path(__file__).parent.parent -class TestCheckDistribution: - def test_source_code(self): - """Directly check the source code in the working directory.""" - src_txt_path = PROJECT_ROOT / "src/pymatgen.egg-info/SOURCES.txt" +def test_source_code(): + """Check the source code in the working directory.""" + src_txt_path = PROJECT_ROOT / "src/pymatgen.egg-info/SOURCES.txt" - _check_src_txt_is_complete(PROJECT_ROOT, src_txt_path) + _check_src_txt_is_complete(PROJECT_ROOT, src_txt_path) - def test_source_distribution(self): - """Build the source distribution and verify its contents.""" - with ScratchDir("."): - # Build the source distribution - subprocess.run(["python", "-m", "pip", "install", "--upgrade", "build"], check=True) - subprocess.run(["python", "-m", "build", "--sdist", PROJECT_ROOT, "--outdir", ".", "-C--quiet"], check=True) - - # Decompress sdist - sdist_file = next(Path(".").glob("*.tar.gz")) - sdist_dir = sdist_file.name.removesuffix(".tar.gz") - with tarfile.open(sdist_file, "r:gz") as tar: - # TODO: remove attr check after only 3.12+ - if hasattr(tarfile, "data_filter"): - tar.extractall("", filter="data") - else: - tar.extractall("") # noqa: S202 - - # Verify source distribution contents - src_txt_path = f"{sdist_dir}/src/pymatgen.egg-info/SOURCES.txt" - _check_src_txt_is_complete(project_root=sdist_dir, src_txt_path=src_txt_path) +def test_build_source_distribution(): + """Test build the source distribution (sdist).""" + with ScratchDir("."): + # Build the source distribution + subprocess.run(["python", "-m", "pip", "install", "--upgrade", "build"], check=True) + subprocess.run(["python", "-m", "build", "--sdist", PROJECT_ROOT, "--outdir", ".", "-C--quiet"], check=True) def _check_src_txt_is_complete(project_root: PathLike, src_txt_path: PathLike) -> None: From 8f291d89227d8a01fd87ca466c8300c091109e8e Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Mon, 25 Nov 2024 23:22:52 +0800 Subject: [PATCH 23/25] also check typed file --- tests/test_pkg.py | 46 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/tests/test_pkg.py b/tests/test_pkg.py index 26b5c8f8b8a..818b668b458 100644 --- a/tests/test_pkg.py +++ b/tests/test_pkg.py @@ -6,6 +6,8 @@ from __future__ import annotations import subprocess +import tarfile +import warnings from glob import glob from pathlib import Path from typing import TYPE_CHECKING @@ -24,14 +26,30 @@ def test_source_code(): _check_src_txt_is_complete(PROJECT_ROOT, src_txt_path) + # Check existence of `py.typed` file in each sub-package + _check_py_typed_files(PROJECT_ROOT / "src/pymatgen") + def test_build_source_distribution(): - """Test build the source distribution (sdist).""" + """Test build the source distribution (sdist), also check `py.typed` files.""" with ScratchDir("."): # Build the source distribution subprocess.run(["python", "-m", "pip", "install", "--upgrade", "build"], check=True) subprocess.run(["python", "-m", "build", "--sdist", PROJECT_ROOT, "--outdir", ".", "-C--quiet"], check=True) + # Decompress sdist + sdist_file = next(Path(".").glob("*.tar.gz")) + sdist_dir = Path(sdist_file.name.removesuffix(".tar.gz")) + with tarfile.open(sdist_file, "r:gz") as tar: + # TODO: remove attr check after only 3.12+ + if hasattr(tarfile, "data_filter"): + tar.extractall("", filter="data") + else: + tar.extractall("") # noqa: S202 + + # Check existence of `py.typed` file in each sub-package + _check_py_typed_files(sdist_dir / "src/pymatgen") + def _check_src_txt_is_complete(project_root: PathLike, src_txt_path: PathLike) -> None: """Check that all source code and data files are listed in given SOURCES.txt. @@ -59,3 +77,29 @@ def _check_src_txt_is_complete(project_root: PathLike, src_txt_path: PathLike) - if unix_path not in sources: raise ValueError(f"{unix_path} not found in {src_txt_path}, check package data config") + + +def _check_py_typed_files(pkg_root: PathLike) -> None: + """ + Ensure all sub-packages contain a `py.typed` file. + + Args: + pkg_root (PathLike): Path to the namespace package's root directory. + + Raises: + FileNotFoundError: If any sub-package is missing the `py.typed` file. + """ + if not (pkg_root := Path(pkg_root)).is_dir(): + raise NotADirectoryError(f"Provided path {pkg_root} is not a valid directory.") + + # Iterate through all directories under the namespace package + for sub_pkg in pkg_root.glob("*/"): + sub_pkg_path = Path(sub_pkg) + + # Check for __init__.py to ensure it's a package + if (sub_pkg_path / "__init__.py").exists(): + if not (sub_pkg_path / "py.typed").exists(): + raise FileNotFoundError(f"Missing py.typed in sub-package: {sub_pkg_path}") + + else: + warnings.warn(f"{sub_pkg_path=} doesn't have __init__.py", stacklevel=2) From 937e54746edcd1028da91bb069c0b986f7b09d5b Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Mon, 25 Nov 2024 23:47:17 +0800 Subject: [PATCH 24/25] temp save --- tests/test_pkg.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/test_pkg.py b/tests/test_pkg.py index 818b668b458..7455e97cbdf 100644 --- a/tests/test_pkg.py +++ b/tests/test_pkg.py @@ -7,7 +7,6 @@ import subprocess import tarfile -import warnings from glob import glob from pathlib import Path from typing import TYPE_CHECKING @@ -18,6 +17,7 @@ from pymatgen.util.typing import PathLike PROJECT_ROOT = Path(__file__).parent.parent +NAMESPACE_PKGS = {"analysis", "ext", "io"} # TODO: double check def test_source_code(): @@ -94,12 +94,14 @@ def _check_py_typed_files(pkg_root: PathLike) -> None: # Iterate through all directories under the namespace package for sub_pkg in pkg_root.glob("*/"): - sub_pkg_path = Path(sub_pkg) + if sub_pkg.name.startswith(".") or sub_pkg.name.startswith("__"): + continue - # Check for __init__.py to ensure it's a package + # Check for __init__.py to ensure it's not a namespace package + sub_pkg_path = Path(sub_pkg) if (sub_pkg_path / "__init__.py").exists(): if not (sub_pkg_path / "py.typed").exists(): raise FileNotFoundError(f"Missing py.typed in sub-package: {sub_pkg_path}") - else: - warnings.warn(f"{sub_pkg_path=} doesn't have __init__.py", stacklevel=2) + elif sub_pkg_path.name not in NAMESPACE_PKGS: + raise ValueError(f"Unexpected namespace package {sub_pkg_path.name}") From 75c9917e8417609ee742850dc85f9689823a22c3 Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Tue, 26 Nov 2024 10:56:47 +0800 Subject: [PATCH 25/25] revert py.typed change --- src/pymatgen/analysis/py.typed | 0 src/pymatgen/apps/py.typed | 0 src/pymatgen/cli/py.typed | 0 src/pymatgen/command_line/py.typed | 0 src/pymatgen/core/py.typed | 0 src/pymatgen/electronic_structure/py.typed | 0 src/pymatgen/entries/py.typed | 0 src/pymatgen/ext/py.typed | 0 src/pymatgen/io/py.typed | 0 src/pymatgen/optimization/py.typed | 0 src/pymatgen/phonon/py.typed | 0 src/pymatgen/{alchemy => }/py.typed | 0 src/pymatgen/symmetry/py.typed | 0 src/pymatgen/transformations/py.typed | 0 src/pymatgen/util/py.typed | 0 src/pymatgen/vis/py.typed | 0 tests/test_pkg.py | 120 +++++---------------- 17 files changed, 24 insertions(+), 96 deletions(-) delete mode 100644 src/pymatgen/analysis/py.typed delete mode 100644 src/pymatgen/apps/py.typed delete mode 100644 src/pymatgen/cli/py.typed delete mode 100644 src/pymatgen/command_line/py.typed delete mode 100644 src/pymatgen/core/py.typed delete mode 100644 src/pymatgen/electronic_structure/py.typed delete mode 100644 src/pymatgen/entries/py.typed delete mode 100644 src/pymatgen/ext/py.typed delete mode 100644 src/pymatgen/io/py.typed delete mode 100644 src/pymatgen/optimization/py.typed delete mode 100644 src/pymatgen/phonon/py.typed rename src/pymatgen/{alchemy => }/py.typed (100%) delete mode 100644 src/pymatgen/symmetry/py.typed delete mode 100644 src/pymatgen/transformations/py.typed delete mode 100644 src/pymatgen/util/py.typed delete mode 100644 src/pymatgen/vis/py.typed diff --git a/src/pymatgen/analysis/py.typed b/src/pymatgen/analysis/py.typed deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/src/pymatgen/apps/py.typed b/src/pymatgen/apps/py.typed deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/src/pymatgen/cli/py.typed b/src/pymatgen/cli/py.typed deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/src/pymatgen/command_line/py.typed b/src/pymatgen/command_line/py.typed deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/src/pymatgen/core/py.typed b/src/pymatgen/core/py.typed deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/src/pymatgen/electronic_structure/py.typed b/src/pymatgen/electronic_structure/py.typed deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/src/pymatgen/entries/py.typed b/src/pymatgen/entries/py.typed deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/src/pymatgen/ext/py.typed b/src/pymatgen/ext/py.typed deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/src/pymatgen/io/py.typed b/src/pymatgen/io/py.typed deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/src/pymatgen/optimization/py.typed b/src/pymatgen/optimization/py.typed deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/src/pymatgen/phonon/py.typed b/src/pymatgen/phonon/py.typed deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/src/pymatgen/alchemy/py.typed b/src/pymatgen/py.typed similarity index 100% rename from src/pymatgen/alchemy/py.typed rename to src/pymatgen/py.typed diff --git a/src/pymatgen/symmetry/py.typed b/src/pymatgen/symmetry/py.typed deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/src/pymatgen/transformations/py.typed b/src/pymatgen/transformations/py.typed deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/src/pymatgen/util/py.typed b/src/pymatgen/util/py.typed deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/src/pymatgen/vis/py.typed b/src/pymatgen/vis/py.typed deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/tests/test_pkg.py b/tests/test_pkg.py index 7455e97cbdf..c47516a5796 100644 --- a/tests/test_pkg.py +++ b/tests/test_pkg.py @@ -1,107 +1,35 @@ -"""Check the content of source code and test build source distribution. - -Binary distribution (wheel) is checked in test workflow. -""" - from __future__ import annotations -import subprocess -import tarfile +import os from glob import glob -from pathlib import Path -from typing import TYPE_CHECKING - -from monty.tempfile import ScratchDir - -if TYPE_CHECKING: - from pymatgen.util.typing import PathLike - -PROJECT_ROOT = Path(__file__).parent.parent -NAMESPACE_PKGS = {"analysis", "ext", "io"} # TODO: double check - - -def test_source_code(): - """Check the source code in the working directory.""" - src_txt_path = PROJECT_ROOT / "src/pymatgen.egg-info/SOURCES.txt" - - _check_src_txt_is_complete(PROJECT_ROOT, src_txt_path) - - # Check existence of `py.typed` file in each sub-package - _check_py_typed_files(PROJECT_ROOT / "src/pymatgen") - - -def test_build_source_distribution(): - """Test build the source distribution (sdist), also check `py.typed` files.""" - with ScratchDir("."): - # Build the source distribution - subprocess.run(["python", "-m", "pip", "install", "--upgrade", "build"], check=True) - subprocess.run(["python", "-m", "build", "--sdist", PROJECT_ROOT, "--outdir", ".", "-C--quiet"], check=True) - # Decompress sdist - sdist_file = next(Path(".").glob("*.tar.gz")) - sdist_dir = Path(sdist_file.name.removesuffix(".tar.gz")) - with tarfile.open(sdist_file, "r:gz") as tar: - # TODO: remove attr check after only 3.12+ - if hasattr(tarfile, "data_filter"): - tar.extractall("", filter="data") - else: - tar.extractall("") # noqa: S202 +import pytest - # Check existence of `py.typed` file in each sub-package - _check_py_typed_files(sdist_dir / "src/pymatgen") +SRC_TXT_PATH = "src/pymatgen.egg-info/SOURCES.txt" -def _check_src_txt_is_complete(project_root: PathLike, src_txt_path: PathLike) -> None: - """Check that all source code and data files are listed in given SOURCES.txt. +@pytest.mark.skipif( + not os.path.isfile(SRC_TXT_PATH), + reason=f"{SRC_TXT_PATH=} not found. Run `pip install .` to create", +) +def test_egg_sources_txt_is_complete(): + """Check that all source and data files in pymatgen/ are listed in pymatgen.egg-info/SOURCES.txt.""" - Args: - project_root (PathLike): Path to the directory containing "src/pymatgen/". - src_txt_path (PathLike): Path to the "SOURCES.txt" file. - """ - project_root = Path(project_root) - src_txt_path = Path(src_txt_path) + with open(SRC_TXT_PATH, encoding="utf-8") as file: + sources = file.read() - assert project_root.is_dir(), f"{project_root} is not a directory" - assert src_txt_path.is_file(), f"{src_txt_path} doesn't exist" - - sources = src_txt_path.read_text(encoding="utf-8") - - # Check that all files listed in "SOURCES.txt" exist + # check that all files listed in SOURCES.txt exist for src_file in sources.splitlines(): - assert (project_root / src_file).is_file(), f"{src_file!r} does not exist!" - - # Check that all files in src/pymatgen/ are listed in SOURCES.txt - for ext in ("py", "json", "json.*", "yaml", "csv"): - for filepath in glob(f"{project_root}/src/pymatgen/**/*.{ext}", recursive=True): - unix_path = Path(filepath).relative_to(project_root).as_posix() - + assert os.path.isfile(src_file), f"{src_file!r} does not exist!" + + # check that all files in pymatgen/ are listed in SOURCES.txt + for ext in ("py", "json*", "yaml", "csv"): + for filepath in glob(f"pymatgen/**/*.{ext}", recursive=True): + unix_path = filepath.replace("\\", "/") + if unix_path.endswith("dao.py"): + continue if unix_path not in sources: - raise ValueError(f"{unix_path} not found in {src_txt_path}, check package data config") - - -def _check_py_typed_files(pkg_root: PathLike) -> None: - """ - Ensure all sub-packages contain a `py.typed` file. - - Args: - pkg_root (PathLike): Path to the namespace package's root directory. - - Raises: - FileNotFoundError: If any sub-package is missing the `py.typed` file. - """ - if not (pkg_root := Path(pkg_root)).is_dir(): - raise NotADirectoryError(f"Provided path {pkg_root} is not a valid directory.") - - # Iterate through all directories under the namespace package - for sub_pkg in pkg_root.glob("*/"): - if sub_pkg.name.startswith(".") or sub_pkg.name.startswith("__"): - continue - - # Check for __init__.py to ensure it's not a namespace package - sub_pkg_path = Path(sub_pkg) - if (sub_pkg_path / "__init__.py").exists(): - if not (sub_pkg_path / "py.typed").exists(): - raise FileNotFoundError(f"Missing py.typed in sub-package: {sub_pkg_path}") - - elif sub_pkg_path.name not in NAMESPACE_PKGS: - raise ValueError(f"Unexpected namespace package {sub_pkg_path.name}") + raise ValueError( + f"{unix_path} not found in {SRC_TXT_PATH}. check setup.py package_data for " + "outdated inclusion rules." + )