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

dvc: update file utilities #2137

Closed
1 of 2 tasks
Suor opened this issue Jun 17, 2019 · 3 comments · Fixed by #3093
Closed
1 of 2 tasks

dvc: update file utilities #2137

Suor opened this issue Jun 17, 2019 · 3 comments · Fixed by #3093
Labels
good first issue p3-nice-to-have It should be done this or next sprint refactoring Factoring and re-factoring

Comments

@Suor
Copy link
Contributor

Suor commented Jun 17, 2019

Some file utilities live in dvc.utils.fs while some other in dvc.utils.__init__. I suggest grouping them in a single place.

Also, some of them accept Path-like objects, i.e. Path, PurePath, PathInfo, etc and some do not. This makes people guess at the call site whether they need to wrap arguments into fspath(), fspath_py35() or none. So I propose that our file utilities will always accept that, this way we will concentrate those wrapper call in one place and remove the guessing part, at least for our own functions, calling os.path.*() will still need wrapping as long as we use Python 3.5-.

@efiop efiop added p2-medium Medium priority, should be done, but less important refactoring Factoring and re-factoring labels Jun 17, 2019
@efiop efiop added p4-not-important and removed p2-medium Medium priority, should be done, but less important labels Jul 23, 2019
@efiop efiop added p3-nice-to-have It should be done this or next sprint and removed p4 labels Sep 25, 2019
@efiop
Copy link
Contributor

efiop commented Oct 25, 2019

For the record, this one is taken by @algomaster99 . Not able to assign yet, working on adding him to organization.

@ghost
Copy link

ghost commented Jan 1, 2020

@efiop , @skshetry is this done?

@efiop
Copy link
Contributor

efiop commented Jan 1, 2020

@MrOutis Not yet, as you can see there are still things like makedirs in __init__.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue p3-nice-to-have It should be done this or next sprint refactoring Factoring and re-factoring
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants