[Build] Run pyright against modified files by hughperkins · Pull Request #14 · Genesis-Embodied-AI/quadrants

hughperkins · 2025-06-05T02:00:19Z

Issue: #

Brief Summary

Run pyright against modified files

first runs pybind11-stubgen
for detecting modified files, uses an AI generated script, in python/tools/run_modified_files.py

copilot:summary

Walkthrough

copilot:walkthrough

CharlesMasson · 2025-06-06T07:02:36Z

+
+set -ex
+
+python python/tools/run_modified_files.py --include 'python/*.py' -- pyright 


We can have errors in unmodified files due to modifications elsewhere, can't we?
Is it particularly slow to run everywhere? Otherwise, I'd keep things simple and run it everywhere, or would parallelize instead.

Its' super fast. Speed is not an issue. The 1500 errors that pop up when we run it are.

See https://genesis-ai-company.slack.com/archives/C08JT0YHM2A/p1748974351128939 for more context

Does it keep checking files that have been changed previously, therefore have been fixed, but have not been changed again in the current commit/PR?

it checks files that have been modified relative to main.

it won't check files that have been modified previously in main, but not since.

basically, it proects against breaking new files, and will gradually enforce pyright on any files, as we modify them (and once a file is fixed, the only way to unfix it would be to modify it again, I think?)

I think that you can cause typing errors in that file if you modify a dependency in another file (e.g., a type that it uses that is defined in another file). And once a file is fixed, I think we should keep running pyright on it.

An alternative may be to maintain a list of exclusions in the pyright configuration from which we'd gradually remove files, as done in gs-core.

right now, that would be a list of like 1500 files... Baby steps. We don't even know we can fix even a single file yet.

I agree with Charles. Not a huge fan of the approach of only checking modified files. But maybe we should remove all this confusing / obsfucated logic once Taichi repo is finally clean.

For sure yeah. I just can't do everything all at once :)

that would be a list of like 1500 files...

We can also add full directory exclusions, if that is more manageable.

CharlesMasson · 2025-06-06T07:03:33Z

+pip3 install scikit-build
+
+pip install pyright pybind11-stubgen


Can we configure this in a pyproject.toml with a dev dependency group so that everyone can use it to set up local dev as well?

I dont think we can use pyproject.toml, though perhaps you know how to configure taichi so we can? Would love to migraate to pyproject.toml

But yes, should probalby add to requirements_dev.txt. I'll do that

Added to reuiqrements_dev.txt

Hmmm, I guess since I'm running a full build anyway, not much excuse for not removing it from the pyright script. I'll try doing that.

CharlesMasson · 2025-06-06T07:08:17Z

+import fnmatch
+
+
+def get_changed_files(include_pattern: str) -> List[str]:


I have mixed feelings about adding this complexity. If it's too slow to run everywhere, couldn't we scope actions (e.g., by directory) and trigger only on directory changes. This would seem a more robust and transparent solution to me.

See other thread.

…p/pyright

duburcqa · 2025-06-09T20:39:05Z

+    steps:
+      # looks like we need to first build the dll...
+      # easiest way to do that is to build the wheel
+      # at some point, we should probably have a separate wheel building thing


I did this for Jiminy in the past. You can find how to do this here.

yeah, lets have that as a separate pr please. One step at a time.

No problem!

duburcqa · 2025-06-09T20:40:00Z

+  build:
+    runs-on: ubuntu-22.04
+    steps:
+      # looks like we need to first build the dll...


Actually you should build the DLL, then generate the stubs to get typing working. This is basically what you did actually, and yes it is absolutely necessary. Note that you should bundle the stubs with the wheel for the end-user (actually a developper using taichi) to have proper typing experience out-of-the-box. I did this before for Jiminy here.

Ok, so the pyi bit should be part of the wheel build? Sounds good. Thanks!

CharlesMasson

As discussed in comments, I don't find that way we currently run pyright very practical, but if we want to address that in a subsequent PR, we can move forward.

Convert MatrixField.to_torch / to_numpy to thin calls into _interop.get_zerocopy_torch / get_zerocopy_numpy. Adds a single _matrix_view_shape() helper that returns (expected_shape, as_vector); both to_torch and to_numpy now go through it instead of duplicating the n / m / keep_dims branching (closes review #13, #14). Same wins as the ScalarField migration: - _zerocopy_cache as cached_property (closes review #17) - automatic registration with pyquadrants.cache_holders (closes #18) - copy=True now zerocopies + clones (closes #15, #16, #21) - to_numpy(copy=False) goes through numpy.from_dlpack directly, no torch round-trip (closes #6) - Apple Metal double-sync via the centralised helpers (closes #1, #22, #23) The redundant reshape after zerocopy stays (DLPack returns the field's flat n-d shape; numpy/torch users expect the matrix dims appended) but it's now in one place per method instead of two.

hughperkins added 4 commits June 4, 2025 17:00

Merge branch 'main' of github.com:genesis-company/taichi

ecc6558

add pyright linter, and run_modified_files.py

7435081

trigger on pr

3e4571b

Merge branch 'main' into hp/pyright

9d0431b

CharlesMasson reviewed Jun 6, 2025

View reviewed changes

hughperkins added 5 commits June 6, 2025 08:54

add pybind11_stubgen to requirements_dev.txt

e1afd94

Merge branch 'hp/pyright' of github.com:genesis-company/taichi into h…

e4cfe92

…p/pyright

add pyright

7f59f71

try removing pyright from prerequiesitse

7a820b7

try installing reuiqrmenets_dev.txt

dad71a6

duburcqa reviewed Jun 9, 2025

View reviewed changes

Comment thread tests/python/test_tools_run_modified_files_integration.py Outdated

duburcqa reviewed Jun 9, 2025

View reviewed changes

Comment thread tests/python/test_tools_run_modified_files_integration.py Outdated

hughperkins added 5 commits June 9, 2025 15:45

Merge branch 'main' into hp/pyright

fef207a

make unit tests not change python cwd

27b3d4b

import tools the convoluted way

b7d01e6

add name to status check

bfae031

fix tests for modified files

2738840

CharlesMasson approved these changes Jun 10, 2025

View reviewed changes

hughperkins added 3 commits June 10, 2025 07:05

Merge branch 'main' into hp/pyright

36d9df5

Merge branch 'main' into hp/pyright

03cac47

precommit

e9deb73

duburcqa approved these changes Jun 10, 2025

View reviewed changes

hughperkins enabled auto-merge (squash) June 10, 2025 17:14

hughperkins added 2 commits June 10, 2025 13:05

Merge branch 'main' into hp/pyright

061d52b

Merge branch 'main' into hp/pyright

9da4718

hughperkins merged commit 63e537a into main Jun 10, 2025
17 checks passed

hughperkins deleted the hp/pyright branch June 10, 2025 22:00

hughperkins mentioned this pull request Apr 22, 2026

[Perf] Enable zero-copy in to_torch() and to_numpy() #450

Merged


		set -ex

		python python/tools/run_modified_files.py --include 'python/*.py' -- pyright

		pip3 install scikit-build

		pip install pyright pybind11-stubgen

		import fnmatch


		def get_changed_files(include_pattern: str) -> List[str]:

Conversation

hughperkins commented Jun 5, 2025

Brief Summary

Walkthrough

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

duburcqa Jun 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

CharlesMasson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

duburcqa Jun 9, 2025 •

edited

Loading