Skip to content

Fix type subscript on older python versions#15090

Merged
MichaReiser merged 1 commit intoastral-sh:mainfrom
aaron-skydio:patch-1
Dec 21, 2024
Merged

Fix type subscript on older python versions#15090
MichaReiser merged 1 commit intoastral-sh:mainfrom
aaron-skydio:patch-1

Conversation

@aaron-skydio
Copy link
Contributor

@aaron-skydio aaron-skydio commented Dec 21, 2024 •
edited
Loading

Summary

Python versions before Python 3.9 don't have subscriptable containers like list, which is being used in the annotation of get_last_three_path_parts here. I believe Ruff supports Python 3.7 and Python 3.8? In that case this needs to either use the container from the typing module, or use deferred annotations with from __future__ import annotations like I'm proposing here

Introduced by 60a2dc5

Test Plan

Currently the code path in find_ruff_bin that defines get_last_three_path_parts doesn't work on Python 3.8 or 3.9, it throws this:

>>> ruff.__main__.find_ruff_bin()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/aaron/miniconda3/envs/symforce/lib/python3.8/site-packages/ruff/__main__.py", line 77, in find_ruff_bin
    raise FileNotFoundError(scripts_path)
FileNotFoundError: /home/aaron/miniconda3/envs/symforce/bin/ruff

With this change it does work

@aaron-skydio
Fix type subscript on older python versions
949275b 
Python versions before Python 3.9 don't have subscriptable containers like `list`, which is being used in the annotation of `get_last_three_path_parts` here.  I believe Ruff supports Python 3.7 and Python 3.8?  In that case this needs to either use the container from the `typing` module, or use deferred annotations with `from __future__ import annotations` like I'm proposing here
@github-actions
Copy link
Contributor

github-actions bot commented Dec 21, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@MichaReiser
Copy link
Member

MichaReiser commented Dec 21, 2024 •
edited
Loading

Thanks for PRing this fix

@charliermarsh would you mind taking a look at this PR? I normally would ask Alex but he's out and I'm probably not the best person to review this change :)

@MichaReiser MichaReiser added the bug Something isn't working label Dec 21, 2024
@MichaReiser
Copy link
Member

MichaReiser commented Dec 21, 2024

I guess that's actually fine. Even I understand why this is needed with a summary as good as this one. Thank you

@MichaReiser MichaReiser merged commit fd4bea5 into astral-sh:main Dec 21, 2024
@charliermarsh
Copy link
Member

charliermarsh commented Dec 21, 2024

Yeah this looks correct to me.

@aaron-skydio aaron-skydio deleted the patch-1 branch December 22, 2024 19:38
@BrewTestBot BrewTestBot mentioned this pull request Jan 2, 2025
Merged
@MichaReiser MichaReiser mentioned this pull request May 20, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@aaron-skydio @MichaReiser @charliermarsh