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

File copy might be needlessly complicated #38

Closed
Maetveis opened this issue Mar 27, 2023 · 4 comments
Closed

File copy might be needlessly complicated #38

Maetveis opened this issue Mar 27, 2023 · 4 comments
Assignees

Comments

@Maetveis
Copy link
Contributor

The file copy in rocm-docs core makes use of the handwritten copy_from_package function:
https://github.com/RadeonOpenCompute/rocm-docs-core/blob/e9c03741abeb0f8043943ce65e16e0d37d6d5de5/src/rocm_docs/__init__.py#L310-L333

This is the same as shutil.copytree outside of the custom error message. Is there some hidden problem with the standard library method or can this be simplified?

@Maetveis
Copy link
Contributor Author

I now realize this is probably because importlib.resources doesn't return actual files, but just "Traversable" objects. When the minimum python version is raised 3.9, then this could be simplified as mentioned in the first comment by calling as_file() first.

@lawruble13
Copy link
Collaborator

Yes, that's exactly correct, it is due to how importlib returns the information to us. I'm not inherently opposed to upgrading the python version, although that would be a lot of changes at this point in time.

@Maetveis
Copy link
Contributor Author

I'm not inherently opposed to upgrading the python version, although that would be a lot of changes at this point in time.

I'm not proposing that, I agree it would be hassle for little gain. Python 3.8 is the default in ubuntu 20.04 (3.9 is available in apt, however), just wanted to note that this discussion can be revisited if/when its raised.

@sohaibnd
Copy link
Contributor

Hi @Maetveis, sorry for the very late response. Thanks for the suggestion, I have replaced copy_from_package with shutil.copytree in #1075. Closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants