Skip to content

Fix typing bug for older python versions#3423

Closed
winterchristian wants to merge 1 commit into
spotify:masterfrom
winterchristian:bugfix_typing
Closed

Fix typing bug for older python versions#3423
winterchristian wants to merge 1 commit into
spotify:masterfrom
winterchristian:bugfix_typing

Conversation

@winterchristian
Copy link
Copy Markdown

Description

This fixes a typing bug introduced in #3416 that only affects older python versions (< python3.10).

Motivation and Context

In #3416 the type hint str was changed to str | None. This works fine for all newer python versions >= 3.10. For the older version law uses the typing-extensions library to introduce typing support for older python versions <3.10.
This library does not support the short equivalent str | None instead of Union[str, None]. In this case that one type is None it is also equivalent to Optional[str].

So this PR just replaces str | None with Optional[str].

Have you tested this? If so, how?

As this is a trivial change, I tested it only locally with my setup using python3.9 and python3.13.

@winterchristian winterchristian requested review from a team and dlstadther as code owners May 8, 2026 06:57
Copy link
Copy Markdown
Collaborator

@dlstadther dlstadther left a comment

Choose a reason for hiding this comment

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

Luigi removed official support for 3.9 with 6959c1f so this PR is N/A.

@winterchristian
Copy link
Copy Markdown
Author

winterchristian commented May 11, 2026

Hi @dlstadther your right I missed that. Should I then close this PR, or should it still be merged out of consistency reasons? The whole file uses Union[] and Optional[] instead of |.

@dlstadther
Copy link
Copy Markdown
Collaborator

There are many places in the repo which haven't been moved away from typing.Union and/or typing.Optional (i actually commented a similar mention on another recent PR). These typing updates (moving to the 3.10+ style) can be made in a separate PR.

I'll close this PR since its intention is n/a. Sorry for the confusion.

@dlstadther dlstadther closed this May 11, 2026
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.

2 participants