Skip to content

fix typing issue and black linting #151

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

Closed
wants to merge 2 commits into from
Closed

fix typing issue and black linting #151

wants to merge 2 commits into from

Conversation

vishalvvr
Copy link
Contributor

Note: Code changes in this PR is only in opl/s3_tools.py, other files are formatted with black.

Error:

s3_resource: boto3.resource, bucket_name: str, file_paths: str | List
TypeError: unsupported operand type(s) for |: 'type' and '_SpecialGenericAlias'

How to reproduce

Run function delete_files from opl/s3_tools.py script with python3.9

Fix:

Use typing.Union instead of | because | is not supported in python versions < 3.10

ref: https://docs.python.org/3/library/typing.html#typing.Union

Syntax Python 3.9 and below Python 3.10+ Python 3.12+
Union[str, List[str]] ✅ Supported ✅ Supported ✅ Supported
`str list[str]` ❌ Not supported ✅ Supported

@vishalvvr vishalvvr added the bug Something isn't working label Apr 18, 2025
Copy link
Contributor

@jhutar jhutar left a comment

Choose a reason for hiding this comment

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

Looks good overall, but please separate actual code changes and style fixes to two different commits.

@jhutar
Copy link
Contributor

jhutar commented Apr 22, 2025

Also please review linter issues above if there is something your code change might be causing. Most probably it is just our technical debt though.

@vishalvvr
Copy link
Contributor Author

Thanks @jhutar for the review 🙏
since this branch has gone stale & unnecessary files tags to this PR.
Hence closing this PR and create a new PR#155 with only relevant files changes :)

@vishalvvr vishalvvr closed this May 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants