`pyright` fixes #3777

DanielYang59 · 2024-04-21T12:41:21Z

Summary

pyright fixes, a follow up PR of pyright fixes for ext/io/phonon/symmetry/transformations/util/vis/dev_scripts and improve io.lobster #3757.
Enable pyright in linting workflow and pre-commit.

pymatgen/core/interface.py

pymatgen/analysis/graphs.py

This reverts commit de34eda.

coderabbitai · 2024-04-22T08:48:30Z

Walkthrough

This update focuses on enhancing error handling, initializing variables before use, and refining logic in a Python project. Changes include adding or adjusting timeouts for HTTP requests, initializing variables to prevent bugs, and refining data handling in functions. The modifications aim to improve stability, readability, and maintainability of the codebase, ensuring smoother operations and clearer error reporting.

Changes

Files	Change Summary
`.github/workflows/lint.yml`, `.pre-commit-config.yaml`	Updated linting configurations and pre-commit hooks.
`pymatgen/analysis/...`, `pymatgen/command_line/...`, `pymatgen/core/...`, `pymatgen/electronic_structure/...`	Numerous updates across `pymatgen` submodules to initialize variables, handle exceptions, and refine logical flows.
`pymatgen/ext/...`, `tests/ext/...`	Increased timeout values for HTTP requests in external interfaces and related tests.
`pyproject.toml`, `tasks.py`	Added virtual environment configuration and adjusted timeout settings in task automation scripts.
`dev_scripts/update_pt_data.py`	Increased timeout for HTTP requests in script for updating data.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

DanielYang59 · 2024-04-22T08:55:13Z

@janosh Please review this, all pyright errors are fixed :)

Not sure why pre-commit is failing though (passing locally), also tried to downgrade to previous but still no luck. Noticed a similar report RobertCraigie/pyright-python#173, but not replied.

janosh

this is great, thanks so much! @ml-evs will be happy to hear this :)

i think pyright is failing in pre-commit CI because it can't find the dependencies installed into a virtual env. try adding to pyproject.toml (CC @njzjz)

[tool.pyright]
venv = "pmg"
venvPath = "~/micromamba/envs" # TODO check this path is correct, not sure where micromamba puts envs

DanielYang59 · 2024-04-22T12:57:27Z

Thanks for the input. I'll have a try.

Just tried to install micromamba locally, this path seems correct.

#3777 (comment) This has to be confirmed :)

janosh · 2024-04-22T13:13:49Z

sorry, actually this was a stupid suggestion. i thought pyright was failing in GitHub action but actually it's only failing in pre-commit CI which runs in a completely different container where pymatgen deps are not installed anyway. we could manually install all of them but that would be tedious to maintain as deps change. better to only run pyright in GitHub Action and skip it in pre-commit CI imo

DanielYang59 · 2024-04-22T13:15:27Z

It's weird, pyright still failed. But I think pyright worked fine in #3757? Back that time we already have micromamba installed in #3729?

This reverts commit f9a3aeb.

njzjz · 2024-04-22T20:46:31Z

Not sure why pre-commit is failing though (passing locally), also tried to downgrade to previous but still no luck. Noticed a similar report RobertCraigie/pyright-python#173, but not replied.

I think the reason is that pre-commit.ci disallows the network at the runtime (see pre-commit-ci/issues#196 (comment)), but pyright-python only downloads pyright at the first runtime (see RobertCraigie/pyright-python#225).

janosh · 2024-04-22T21:34:20Z

thanks @njzjz! that's good to know. more reason to skip it in pre-commit CI.

@DanielYang59 could you add pyright here?

pymatgen/.pre-commit-config.yaml

Line 5 in ad6eafe

skip: [mypy]

DanielYang59 · 2024-04-23T01:01:21Z

Thanks @njzjz! It's very helpful.

DanielYang59 · 2024-04-23T01:04:43Z

@janosh. Yes it makes sense, we may not need pyright on both CI and pre-commit. Skipped in pre-commit, thanks!

DanielYang59 · 2024-04-23T01:13:32Z

Ah, wait! I'm pretty sure I added a timeout of 60 seconds for request to prevent it from hanging forever, but it seems 60 seconds is insufficient?

self = <ext.test_cod.TestCOD testMethod=test_get_structure_by_id>

    def test_get_structure_by_id(self):
>       struct = COD().get_structure_by_id(2002926)

tests/ext/test_cod.py:31: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
pymatgen/ext/cod.py:93: in get_structure_by_id
    response = requests.get(f"http://{self.url}/cod/{cod_id}.cif", timeout=60)
../../../micromamba/envs/pmg/lib/python3.11/site-packages/requests/api.py:73: in get
    return request("get", url, params=params, **kwargs)
../../../micromamba/envs/pmg/lib/python3.11/site-packages/requests/api.py:59: in request
    return session.request(method=method, url=url, **kwargs)
../../../micromamba/envs/pmg/lib/python3.11/site-packages/requests/sessions.py:589: in request
    resp = self.send(prep, **send_kwargs)
../../../micromamba/envs/pmg/lib/python3.11/site-packages/requests/sessions.py:703: in send
    r = adapter.send(request, **kwargs)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <requests.adapters.HTTPAdapter object at 0x7f3737f1e250>
request = <PreparedRequest [GET]>, stream = False
timeout = Timeout(connect=60, read=60, total=None), verify = True, cert = None
proxies = OrderedDict()

janosh · 2024-04-23T01:39:17Z

feel free to skip the COD test as well. pinging live APIs in CI is bad practice anyway

coderabbitai

Actionable comments posted: 0

Out of diff range and nitpick comments (1)

pymatgen/ext/cod.py (1)

60-60: Consider addressing the TODO to remove dependency on external mysql call.

DanielYang59 · 2024-04-23T02:19:46Z

Thanks for reviewing @janosh.

Also ping @ml-evs: all pyright errors have been fixed :)

DanielYang59 · 2024-04-23T13:15:59Z

@janosh. I notice CI is failing after this PR (more specifically after the merge master commit a297ef8). It's also failing locally. Did we actually break anything?

UPDATE: I just opened #3781 to fix this. Seems like a typo in #3771?

janosh · 2024-04-23T13:36:24Z

thanks for the quick fix!

janosh · 2024-04-23T13:40:28Z

btw, very happy that we can enable pyright's possibly unbound variable detection across the code base thanks to this PR. that will really help automated code quality assurance

DanielYang59 · 2024-04-23T13:43:20Z

btw, very happy that we can enable pyright's possibly unbound variable detection across the code base thanks to this PR. that will really help automated code quality assurance

Great to know that! And very glad I could help :)

DanielYang59 added 8 commits April 21, 2024 20:39

enable pyright

2075587

fix electronic_structure.plotters

7fbf88b

update pre-commit

13810cb

finish electronic_structure

313f895

fix core.surface/tensors/tracjectory

77f26a4

fix unit test

e58df83

fix core.surface

20a83b1

add DEBUG tag

06a05e8

DanielYang59 commented Apr 22, 2024

View reviewed changes

pymatgen/core/interface.py Outdated Show resolved Hide resolved

DanielYang59 added 4 commits April 22, 2024 10:24

fix core.interface

d6adec2

fix command_line

c34e716

fix some analysis

6034ac7

fix some analysis

464077f

DanielYang59 commented Apr 22, 2024

View reviewed changes

pymatgen/analysis/graphs.py Show resolved Hide resolved

DanielYang59 added 8 commits April 22, 2024 11:37

fix more analysis

54a5a2d

fix np.empty

83cb871

fix more analysis

c21d454

fix chemenv.utils

d7cfa9f

finish chemenv

6e2b326

revert changes that break tests

de34eda

Revert "revert changes that break tests"

65e6ca4

This reverts commit de34eda.

fix unit test

48f87f5

DanielYang59 marked this pull request as ready for review April 22, 2024 08:48

DanielYang59 requested review from shyuep, janosh and mkhorton as code owners April 22, 2024 08:48

DanielYang59 added 2 commits April 22, 2024 16:52

remove tag

b2e86ac

remove TODO tag

54818d1

janosh reviewed Apr 22, 2024

View reviewed changes

add venv path for pyright

f9a3aeb

Revert "add venv path for pyright"

5c08729

This reverts commit f9a3aeb.

DanielYang59 added 2 commits April 23, 2024 09:01

Merge branch 'master' into pyright

e15186c

skip pyright in pre-commit

381f430

increase requests timeout from 60s to 600s

34be0ff

skip tests/ext/test_cod.py in CI

bca91fe

janosh enabled auto-merge (squash) April 23, 2024 01:51

coderabbitai bot reviewed Apr 23, 2024

View reviewed changes

janosh and others added 3 commits April 22, 2024 18:56

fix plot_slab weird way to calc zorder

556e897

internal types

34aaa2c

Merge branch 'master' into pyright

a297ef8

janosh disabled auto-merge April 23, 2024 02:18

janosh merged commit 1367746 into materialsproject:master Apr 23, 2024
20 of 22 checks passed

DanielYang59 deleted the pyright branch April 23, 2024 02:19

chiang-yuan mentioned this pull request May 4, 2024

bugfix: chargemol runtime directory and os.path.exists fix #3778

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`pyright` fixes #3777

`pyright` fixes #3777

DanielYang59 commented Apr 21, 2024 •

edited

Loading

coderabbitai bot commented Apr 22, 2024 •

edited

Loading

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (`.coderabbit.yaml`)

Documentation and Community

DanielYang59 commented Apr 22, 2024 •

edited

Loading

janosh left a comment •

edited

Loading

DanielYang59 commented Apr 22, 2024 •

edited

Loading

janosh commented Apr 22, 2024 •

edited

Loading

DanielYang59 commented Apr 22, 2024 •

edited

Loading

njzjz commented Apr 22, 2024 •

edited

Loading

janosh commented Apr 22, 2024

DanielYang59 commented Apr 23, 2024

DanielYang59 commented Apr 23, 2024

DanielYang59 commented Apr 23, 2024 •

edited by janosh

Loading

janosh commented Apr 23, 2024

coderabbitai bot left a comment

DanielYang59 commented Apr 23, 2024

DanielYang59 commented Apr 23, 2024 •

edited

Loading

janosh commented Apr 23, 2024

janosh commented Apr 23, 2024

DanielYang59 commented Apr 23, 2024

pyright fixes #3777

pyright fixes #3777

Conversation

DanielYang59 commented Apr 21, 2024 • edited Loading

Summary

coderabbitai bot commented Apr 22, 2024 • edited Loading

Walkthrough

Changes

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (.coderabbit.yaml)

Documentation and Community

DanielYang59 commented Apr 22, 2024 • edited Loading

janosh left a comment • edited Loading

Choose a reason for hiding this comment

DanielYang59 commented Apr 22, 2024 • edited Loading

janosh commented Apr 22, 2024 • edited Loading

DanielYang59 commented Apr 22, 2024 • edited Loading

njzjz commented Apr 22, 2024 • edited Loading

janosh commented Apr 22, 2024

DanielYang59 commented Apr 23, 2024

DanielYang59 commented Apr 23, 2024

DanielYang59 commented Apr 23, 2024 • edited by janosh Loading

janosh commented Apr 23, 2024

coderabbitai bot left a comment

Choose a reason for hiding this comment

DanielYang59 commented Apr 23, 2024

DanielYang59 commented Apr 23, 2024 • edited Loading

janosh commented Apr 23, 2024

janosh commented Apr 23, 2024

DanielYang59 commented Apr 23, 2024

`pyright` fixes #3777

`pyright` fixes #3777

DanielYang59 commented Apr 21, 2024 •

edited

Loading

coderabbitai bot commented Apr 22, 2024 •

edited

Loading

CodeRabbit Configration File (`.coderabbit.yaml`)

DanielYang59 commented Apr 22, 2024 •

edited

Loading

janosh left a comment •

edited

Loading

DanielYang59 commented Apr 22, 2024 •

edited

Loading

janosh commented Apr 22, 2024 •

edited

Loading

DanielYang59 commented Apr 22, 2024 •

edited

Loading

njzjz commented Apr 22, 2024 •

edited

Loading

DanielYang59 commented Apr 23, 2024 •

edited by janosh

Loading

DanielYang59 commented Apr 23, 2024 •

edited

Loading