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

fs: path: use flavour.basename #7764

Merged
merged 1 commit into from May 17, 2022
Merged

fs: path: use flavour.basename #7764

merged 1 commit into from May 17, 2022

Conversation

dtrifiro
Copy link
Contributor

calling parts() to get name requires a lot of unnecessary splitting, which quickly adds up when dealing with large datasets.

@dtrifiro dtrifiro requested a review from a team as a code owner May 17, 2022 11:48
@dtrifiro dtrifiro requested a review from karajan1001 May 17, 2022 11:48
@dtrifiro

This comment was marked as outdated.

Sign in to view
@dtrifiro dtrifiro added the performance improvement over resource / time consuming tasks label May 17, 2022
@efiop
Copy link
Member

efiop commented May 17, 2022 •
edited

Btw, alternative might be to make parts a generator, but that would require a bit more changes around the code base, but probably a long term correct solution. Though might be unnecessary for now.

efiop
efiop reviewed May 17, 2022
View reviewed changes
dvc/objects/fs/path.py Outdated Show resolved Hide resolved
@dtrifiro dtrifiro marked this pull request as draft May 17, 2022 13:39
@dtrifiro dtrifiro changed the title objects.fs: use rsplit in Path.name objects.fs: use split in Path.name May 17, 2022
efiop
efiop reviewed May 17, 2022
View reviewed changes
dvc/objects/fs/path.py Outdated Show resolved Hide resolved
@efiop efiop force-pushed the fix/speed-up-path branch 4 times, most recently from dd7cec9 to a10269b Compare May 17, 2022 16:58
@efiop efiop marked this pull request as ready for review May 17, 2022 17:06
@efiop efiop merged commit 01cc97f into iterative:main May 17, 2022
@efiop efiop changed the title objects.fs: use split in Path.name fs: path: use flavour.basename May 17, 2022
@dtrifiro dtrifiro deleted the fix/speed-up-path branch May 18, 2022 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@efiop efiop efiop left review comments

@karajan1001 karajan1001 Awaiting requested review from karajan1001 karajan1001 was automatically assigned from iterative/dvc

Assignees
No one assigned
Labels
performance improvement over resource / time consuming tasks
Projects
No open projects
DVC
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@dtrifiro @efiop