Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump min supported Python to 3.10 #195

Merged
merged 14 commits into from
Sep 1, 2024
Merged

Bump min supported Python to 3.10 #195

merged 14 commits into from
Sep 1, 2024

Conversation

janosh
Copy link
Owner

@janosh janosh commented Aug 31, 2024

start testing all make_assets scripts in GitHub Actions in 5b216f1 and so closes #164

@janosh janosh added breaking Breaking changes pkg Package labels Aug 31, 2024
@janosh janosh added the ci Continuous integration label Sep 1, 2024
@DanielYang59
Copy link
Collaborator

DanielYang59 commented Sep 1, 2024

Perhaps we should also add a py.typed marker to include type information into the package?

Package maintainers who wish to support type checking of their code MUST add a marker file named py.typed to their package supporting typing. This marker applies recursively: if a top-level package includes it, all its sub-packages MUST support type checking as well. To have this file installed with the package, maintainers can use existing packaging options such as package_data in distutils, shown below.

@janosh
Copy link
Owner Author

janosh commented Sep 1, 2024

Perhaps we should also add a py.typed marker to include type information into the package?

sure thing, please do! 👍

@janosh janosh added the types Type all the things label Sep 1, 2024
@janosh
Copy link
Owner Author

janosh commented Sep 1, 2024

i'll make a new release after this PR to test it in production right away

@janosh janosh merged commit 0b9b5da into main Sep 1, 2024
19 checks passed
@janosh janosh deleted the min-py-310 branch September 1, 2024 05:34
@DanielYang59
Copy link
Collaborator

I was wondering why ffonons cannot be installed in CI, but encountered another issue locally. Do I need to install some additional packages beyond those listed in make-assets? Looks like ffonons requires data/phonon-db/map-mp-id-togo-id.csv

(venv) yang@Yang-MacBook make_assets % python3 phonons.py 
Traceback (most recent call last):
  File "/Users/yang/developer/pymatviz/examples/make_assets/phonons.py", line 29, in <module>
    docs[key] = json.loads(file.read(), cls=MontyDecoder)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/[email protected]/3.12.5/Frameworks/Python.framework/Versions/3.12/lib/python3.12/json/__init__.py", line 359, in loads
    return cls(**kw).decode(s)
           ^^^^^^^^^^^^^^^^^^^
  File "/Users/yang/developer/pymatviz/venv/lib/python3.12/site-packages/monty/json.py", line 875, in decode
    return self.process_decoded(d)
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/yang/developer/pymatviz/venv/lib/python3.12/site-packages/monty/json.py", line 785, in process_decoded
    mod = __import__(modname, globals(), locals(), [classname], 0)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/yang/developer/pymatviz/venv/lib/python3.12/site-packages/ffonons/dbs/phonondb.py", line 39, in <module>
    mp_to_togo_id = pd.read_csv(id_map_path, index_col=0)[togo_id_key].to_dict()
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/yang/developer/pymatviz/venv/lib/python3.12/site-packages/pandas/io/parsers/readers.py", line 1026, in read_csv
    return _read(filepath_or_buffer, kwds)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/yang/developer/pymatviz/venv/lib/python3.12/site-packages/pandas/io/parsers/readers.py", line 620, in _read
    parser = TextFileReader(filepath_or_buffer, **kwds)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/yang/developer/pymatviz/venv/lib/python3.12/site-packages/pandas/io/parsers/readers.py", line 1620, in __init__
    self._engine = self._make_engine(f, self.engine)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/yang/developer/pymatviz/venv/lib/python3.12/site-packages/pandas/io/parsers/readers.py", line 1880, in _make_engine
    self.handles = get_handle(
                   ^^^^^^^^^^^
  File "/Users/yang/developer/pymatviz/venv/lib/python3.12/site-packages/pandas/io/common.py", line 873, in get_handle
    handle = open(
             ^^^^^
FileNotFoundError: [Errno 2] No such file or directory: '/Users/yang/developer/pymatviz/venv/lib/python3.12/site-packages/data/phonon-db/map-mp-id-togo-id.csv'

@janosh
Copy link
Owner Author

janosh commented Sep 1, 2024

Looks like ffonons requires data/phonon-db/map-mp-id-togo-id.csv

yeah, that's an issue with the ffonons package which is still very much WIP. will fix that in the next release.

@DanielYang59
Copy link
Collaborator

Thanks for the input. I would add a TODO tag there :)

DanielYang59 added a commit that referenced this pull request Sep 1, 2024
janosh added a commit that referenced this pull request Sep 2, 2024
* ruff fixes

* fix s101 use of assert

* globally ignore SLF001, access private class member

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* add todo tag

* put docstring and global ignore on top

* fix py017, properly capture exception

* Fix PT011, exception capture without match

* add TODO tag for #195 (comment)

* fix NPY201, np trapz

* use warn over print to fix T201

* add comments for globally ignored ruff rules

* guard type check only import when possible

* remove ISC001 from ignore

* add todo tag

* reduce duration items shown to 20

* import Self from typing extension

* remove global ignore of S311 as there're only 2 violation

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* replace random with numpy, which also fixes S311

* fix "PT006",   # pytest-parametrize-names-wrong-type

* guard type check only import

* fix "COM812",  # trailing comma missing

* remove no type check mark

* Revert "fix "COM812",  # trailing comma missing"

This reverts commit 5ab3390.

* fix unit test of io

* NEED confirm: change width from 457 to 458

* clean up branch condition

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* use native array size arg

* revert accidental elem -> element rename

* fix random array generation

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix random array generate in asset maker

* revert test_io value to 457, reason unknown

* skip eslint for now #197

* format pre-commit config

* remove eslint skip mark in CI

* fix indentation in pyproject.toml

* drop quotes in test.yml string

* fix typo in filename

* fix shaded_ys type

* remove overwriting pytest duration

* pyproject whitespace

---------

Co-authored-by: Janosh Riebesell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking changes ci Continuous integration pkg Package types Type all the things
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Dev] Include asset makers into CI tests
2 participants