Skip to content

Conversation

@kevinjqliu
Copy link
Contributor

Rationale for this change

Updated prek libraries (poetry run prek auto-update)
This also triggered updates to deprecate Union and Optional since we no longer support python 3.9
Skipping ruff zip-without-explicit-strict (B905) for now

Are these changes tested?

Yes, make lint

Are there any user-facing changes?

@kevinjqliu kevinjqliu requested a review from Fokko November 4, 2025 03:54
@kevinjqliu kevinjqliu force-pushed the kevinjqliu/prek-update branch 2 times, most recently from bebb44f to 13fca91 Compare November 4, 2025 04:00
@kevinjqliu kevinjqliu changed the title Kevinjqliu/prek update update prek libraries, make lint Nov 4, 2025
@kevinjqliu
Copy link
Contributor Author

CI errors with

#7 ERROR: docker.io/library/openjdk:8-jre-slim: not found

looks like that image is gone from dockerhub. #2697 should fix CI

@kevinjqliu kevinjqliu force-pushed the kevinjqliu/prek-update branch from 13fca91 to 1da2f31 Compare November 4, 2025 15:44
"I", # isort
"UP", # pyupgrade
]
ignore = ["E501","E203","B024","B028","UP037", "UP035", "UP006"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a comment here to revisit the zip behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i coudnt find a good way to add a comment for B905 specifically since there are other ignored rules here.

So i created an issue to track and burn these down
#2700

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for creating the issue. I think the zip should be a good one to get in. Currently, we have checks in place to ensure that the lengths are equivalent, but removing them would be beneficial.

@geruh
Copy link
Contributor

geruh commented Nov 4, 2025

Thanks for raising this Kevin! I saw this happening in the UV migration PR and thought I was missing something.

Looks like this covers PEP 604 which introduces X | Y, which makes int | None exactly equivalent to Optional[int]. The PEP didn’t deprecate Optional type but it left any removal descisions for the future. But looks like ruff kind of nudges everyone in the 604 direction haha.

@kevinjqliu
Copy link
Contributor Author

The PEP didn’t deprecate Optional type but it left any removal descisions for the future

Ah that might explain why ruff left Optionals alont, i had to manually remove them

@Fokko
Copy link
Contributor

Fokko commented Nov 5, 2025

I find Optional[int] easier to read than int | None, or is that just me? :D

@kevinjqliu
Copy link
Contributor Author

I find Optional[int] easier to read than int | None, or is that just me? :D

yea im used to Optional too

@kevinjqliu kevinjqliu merged commit ef5b771 into apache:main Nov 5, 2025
8 checks passed
@kevinjqliu kevinjqliu deleted the kevinjqliu/prek-update branch November 5, 2025 13:26
@kevinjqliu
Copy link
Contributor Author

Thanks for the review @geruh @Fokko

kevinjqliu added a commit that referenced this pull request Nov 6, 2025
…2701)

<!--
Thanks for opening a pull request!
-->

<!-- In the case this PR will resolve an issue, please replace
${GITHUB_ISSUE_ID} below with the actual Github issue id. -->
<!-- Closes #${GITHUB_ISSUE_ID} -->

# Rationale for this change
Removes ignoring ruff rule [(B905
zip-without-explicit-strict)](https://docs.astral.sh/ruff/rules/zip-without-explicit-strict/)
Use `strict=True` in all zip instances

Relates to #2700
Follow up to #2696

## Are these changes tested?
Yes, existing test suite

## Are there any user-facing changes?
No
<!-- In the case of user-facing changes, please add the changelog label.
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants