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

Pull in RustPython parser #6099

Merged
merged 4 commits into from
Jul 27, 2023
Merged

Pull in RustPython parser #6099

merged 4 commits into from
Jul 27, 2023

Conversation

MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Jul 26, 2023

Summary

This PR pulls in the RustPython-parser from https://github.com/astral-sh/RustPython-Parser

The reason for pulling in RustPython are:

  • Working on two repositories requires more steps: 1) Crate a PR to the parser repository and merge, 2) Update the hash in our dependency, fix Ruff
  • Our fork has diverged significantly from upstream, to the point where pulling in changes automatically is hard, and copy pasting the changes is often times easier.

This doesn't mean we won't contribute to the upstream parser in the future. We'll decide on upstream contributions on a case per case basis.

Another benefit of pulling in the rust python AST is that we can now define our own method on the AST types rather than needing free-standing functions or traits.

Requested Feedback

  • I still need to pull in the RustPython license. Let me know if you have opinions on where to place it and whether we should mention the RustPython team in the crates author metadata
  • I copied over the files from the repository. That means we lost the git history. Let me know if you find it important to preserve the history and I can look into it if there's a way to do it
  • Merging rustpython-ast and ruff_python_ast revealed that using the parser inside of ruff_python_ast unit tests is no longer possible because that creates a cyclic dependency. The only way around this is to either extract the tests as integration tests (what I've done here), or keep the two crates separate.

Test Plan

cargo test

@MichaReiser MichaReiser requested a review from dhruvmanila as a code owner July 26, 2023 16:55
@MichaReiser
Copy link
Member Author

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@MichaReiser MichaReiser force-pushed the pull-in-rust-python branch 3 times, most recently from 4b48762 to 6c40a51 Compare July 26, 2023 16:59
@MichaReiser MichaReiser force-pushed the pull-in-rust-python branch from 6c40a51 to 004c8b0 Compare July 27, 2023 06:24
@MichaReiser MichaReiser enabled auto-merge (squash) July 27, 2023 06:24
@MichaReiser MichaReiser added the internal An internal refactor or improvement label Jul 27, 2023
@davidszotten
Copy link
Contributor

seems a shame to lose the history. i think last time i did something like this i followed https://medium.com/@ayushya/move-directory-from-one-repository-to-another-preserving-git-history-d210fa049d4b (i may be misremembering the article; there are lots of tutorials online and i've successfully merged repos a few times in the past without too much trouble)

@MichaReiser MichaReiser force-pushed the pull-in-rust-python branch 3 times, most recently from 013ffcb to 66997bd Compare July 27, 2023 09:15
@MichaReiser MichaReiser force-pushed the pull-in-rust-python branch from 66997bd to 447a923 Compare July 27, 2023 09:18
@MichaReiser MichaReiser merged commit 40f5437 into main Jul 27, 2023
@MichaReiser MichaReiser deleted the pull-in-rust-python branch July 27, 2023 09:29
@github-actions
Copy link
Contributor

github-actions bot commented Jul 27, 2023

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00     11.7±0.48ms     3.5 MB/sec    1.03     12.0±0.44ms     3.4 MB/sec
formatter/numpy/ctypeslib.py               1.00      2.3±0.13ms     7.3 MB/sec    1.03      2.3±0.12ms     7.1 MB/sec
formatter/numpy/globals.py                 1.00   255.2±18.36µs    11.6 MB/sec    1.02   259.1±19.73µs    11.4 MB/sec
formatter/pydantic/types.py                1.00      5.0±0.20ms     5.1 MB/sec    1.01      5.0±0.18ms     5.1 MB/sec
linter/all-rules/large/dataset.py          1.01     17.7±0.56ms     2.3 MB/sec    1.00     17.6±0.69ms     2.3 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      4.3±0.24ms     3.9 MB/sec    1.00      4.3±0.14ms     3.9 MB/sec
linter/all-rules/numpy/globals.py          1.00   565.4±23.93µs     5.2 MB/sec    1.01   571.5±31.48µs     5.2 MB/sec
linter/all-rules/pydantic/types.py         1.00      7.8±0.20ms     3.3 MB/sec    1.00      7.8±0.32ms     3.3 MB/sec
linter/default-rules/large/dataset.py      1.01      8.7±0.22ms     4.7 MB/sec    1.00      8.6±0.26ms     4.7 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1779.1±81.36µs     9.4 MB/sec    1.00  1786.2±57.55µs     9.3 MB/sec
linter/default-rules/numpy/globals.py      1.01   208.1±11.46µs    14.2 MB/sec    1.00   206.4±11.79µs    14.3 MB/sec
linter/default-rules/pydantic/types.py     1.02      3.8±0.12ms     6.6 MB/sec    1.00      3.8±0.13ms     6.7 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00      8.7±0.28ms     4.7 MB/sec    1.02      8.8±0.13ms     4.6 MB/sec
formatter/numpy/ctypeslib.py               1.00  1575.9±17.03µs    10.6 MB/sec    1.04   1639.4±8.07µs    10.2 MB/sec
formatter/numpy/globals.py                 1.00    165.0±3.46µs    17.9 MB/sec    1.01    167.1±4.04µs    17.7 MB/sec
formatter/pydantic/types.py                1.00      3.5±0.04ms     7.2 MB/sec    1.03      3.7±0.03ms     7.0 MB/sec
linter/all-rules/large/dataset.py          1.00     11.8±0.05ms     3.4 MB/sec    1.04     12.3±0.75ms     3.3 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.0±0.02ms     5.5 MB/sec    1.00      3.0±0.03ms     5.5 MB/sec
linter/all-rules/numpy/globals.py          1.01    306.9±2.62µs     9.6 MB/sec    1.00    304.4±5.49µs     9.7 MB/sec
linter/all-rules/pydantic/types.py         1.01      5.2±0.02ms     4.9 MB/sec    1.00      5.2±0.04ms     4.9 MB/sec
linter/default-rules/large/dataset.py      1.00      6.4±0.07ms     6.3 MB/sec    1.01      6.5±0.24ms     6.2 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.01  1233.1±11.82µs    13.5 MB/sec    1.00  1225.3±15.03µs    13.6 MB/sec
linter/default-rules/numpy/globals.py      1.02    126.5±2.71µs    23.3 MB/sec    1.00    123.5±0.92µs    23.9 MB/sec
linter/default-rules/pydantic/types.py     1.05      2.8±0.04ms     9.1 MB/sec    1.00      2.7±0.04ms     9.5 MB/sec

@MichaReiser
Copy link
Member Author

Oh no, auto merge was still enabled. I wanted to look into in preserving the history

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal An internal refactor or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants