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.

Sign up for GitHub

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

Jump to bottom

Use path_isin instead of startswith to compare filepaths #2823

Closed
skshetry opened this issue Nov 20, 2019 · 3 comments · Fixed by #2832
Closed

Use path_isin instead of startswith to compare filepaths #2823

skshetry opened this issue Nov 20, 2019 · 3 comments · Fixed by #2832
Assignees
@skshetry
Labels
enhancement Enhances DVC p2-medium Medium priority, should be done, but less important refactoring Factoring and re-factoring

Comments

@skshetry
Copy link
Member

The PR #2798 introduced dvc.utils.fs.path_isin that can be used to compare child paths and parent paths and therefore, raw string compare with startswith can be replaced with this new function.

Refactor usage of startswith for paths with path_isin.
@triage-new-issues triage-new-issues bot added the triage Needs to be triaged label Nov 20, 2019
@skshetry
Copy link
Member Author

@efiop, can I take this? If yes, please assign me. :)

@efiop
Copy link
Contributor

efiop commented Nov 21, 2019

@skshetry Sure, go ahead 🙂

@skshetry
Copy link
Member Author

Looked through the codebase. The difference between path_isin and startswith is that, the former will be false if they are same path. The latter is being used here in the cases of where the path are same as well. Found 5/6 uses of startswith only.

@efiop efiop added the refactoring Factoring and re-factoring label Nov 21, 2019
@triage-new-issues triage-new-issues bot removed the triage Needs to be triaged label Nov 21, 2019
@efiop efiop added enhancement Enhances DVC triage Needs to be triaged labels Nov 21, 2019
@triage-new-issues triage-new-issues bot removed the triage Needs to be triaged label Nov 21, 2019
@efiop efiop added p2-medium Medium priority, should be done, but less important triage Needs to be triaged labels Nov 21, 2019
@triage-new-issues triage-new-issues bot removed the triage Needs to be triaged label Nov 21, 2019
@skshetry skshetry mentioned this issue Nov 21, 2019
Merged
3 tasks
@weekly-digest weekly-digest bot mentioned this issue Nov 24, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@skshetry skshetry

Labels
enhancement Enhances DVC p2-medium Medium priority, should be done, but less important refactoring Factoring and re-factoring
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Refactor startswith with path_isin util skshetry/dvc
2 participants
@efiop @skshetry