Skip to content
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

[Core feature] Replace os.path with pathlib for flytekit #5384

Open
2 tasks done
MortalHappiness opened this issue May 17, 2024 · 1 comment
Open
2 tasks done

[Core feature] Replace os.path with pathlib for flytekit #5384

MortalHappiness opened this issue May 17, 2024 · 1 comment
Labels
enhancement New feature or request flytekit FlyteKit Python related issue

Comments

@MortalHappiness
Copy link
Member

MortalHappiness commented May 17, 2024

Motivation: Why do you think this is important?

pathlib is a more modern way to do path manipulation since Python 3.4. Currently flytekit mixes usage of os.path and pathlib, which makes the code harder to read.

Goal: What should the final outcome look like, ideally?

Using the correspondance table to replace all os.path with pathlib except for some low level string operations that do not have a counterpart in pathlib. For example, os.path.sep.

Describe alternatives you've considered

Replace all pathlib with os.path. This can also prevent the mixed usage of os.path and pathlib but it is not recommended.

Propose: Link/Inline OR Additional context

No response

Are you sure this issue hasn't been raised already?

  • Yes

Have you read the Code of Conduct?

  • Yes
@MortalHappiness MortalHappiness added enhancement New feature or request untriaged This issues has not yet been looked at by the Maintainers labels May 17, 2024
Copy link

runllm bot commented May 17, 2024

To replace os.path with pathlib in Flytekit, follow the correspondence table to ensure a smooth transition. This will modernize the codebase and improve readability.

Would you like more details on specific replacements or examples of how to implement this change?

Sources:


This is an AI-generated response and your feedback is appreciated! Please leave a 👍 if this is helpful and 👎 if it is not.

Mention @RunLLM to ask follow-up questions. See runllm.com for more!

@pingsutw pingsutw added flytekit FlyteKit Python related issue and removed untriaged This issues has not yet been looked at by the Maintainers labels May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request flytekit FlyteKit Python related issue
Projects
None yet
Development

No branches or pull requests

2 participants