Uh oh!
There was an error while loading. Please reload this page.
Replace pre-commit by prek - #2533
Conversation
80b6728 to
2346c2bCompare2346c2b to
256a370Compareluizvbo
commented
Sep 26, 2025
Closes #2533 |
luizvbo
commented
Sep 26, 2025
Hello! Could you review this PR for me? Thanks in advance |
Fokko
commented
Sep 26, 2025
Thanks for working on this! 🥳
This is not really an issue; however, running it from different machines (and also Dependabot does this) will result in more conflicts. |
luizvbo
commented
Sep 26, 2025
Strange... The tests are working in my machine (python 3.12), but failing in the CI (python 3.11). It's strange, because I didn't touch src. I think it might be caused by updating the packages in the poetry.lock file |
luizvbo
commented
Sep 30, 2025
@Fokko, I think the issue is fixed now (tests are passing locally with py 3.11). I had to set an upper limit for the datafusion package, given that tests fail in py3.11 after version 48. |
Fokko
left a comment
There was a problem hiding this comment.
Nice, this looks great @luizvbo! I'm positive, let's see what @kevinjqliu thinks!
kevinjqliu
left a comment
There was a problem hiding this comment.
LGTM
An improvement in runtime!
prek: make lint 8.10s user 1.18s system 255% cpu 3.638 total
poetry: make lint 11.65s user 1.76s system 171% cpu 7.815 total
| thrift-sasl = { version = ">=0.4.3", optional = true } | ||
| kerberos = {version = "^1.3.1", optional = true} | ||
| datafusion = { version = ">=45", optional = true } | ||
| datafusion = { version = ">=45,<49", optional = true } |
There was a problem hiding this comment.
i tested locally that this passes without the extra <49
poetry lock
make install
poetry run pytest tests/table/test_datafusion.py There was a problem hiding this comment.
its fine to leave as is. due to limitations we actually cannot use datafusion 49, so this makes sense
Uh oh!
There was an error while loading. Please reload this page.
Rationale for this change
It replaces pre-commit with prek, which is almost 10x faster. prek is a port of pre-commit to rust.
Are these changes tested?
Yes.
make lintis running locallyAre there any user-facing changes?
Only for developers, given that it affects
make lint,NOTE: I had to update the poetry.lock file with
poetry lock, resulting in multiple changes. Maybe it's better to run again in your machine (I don't know if it's run automatically in the CI pipeline).