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

Add persistent and overwrite arguments to dataset factory functions #3095

Merged
merged 3 commits into from
Jun 12, 2023

Conversation

swheaton
Copy link
Contributor

@swheaton swheaton commented May 22, 2023

What changes are proposed in this pull request?

I propose adding persistent (default False) and overwrite (default False) arguments to each of the Dataset factory functions (fo.Dataset.from_*), to mirror the parameters available in the fo.Dataset constructor.

Makes the solution for #3016 a little more elegant

For reference, here is the args docstring for class Dataset

    Args:
        name (None): the name of the dataset. By default,
            :func:`get_default_dataset_name` is used
        persistent (False): whether the dataset should persist in the database
            after the session terminates
        overwrite (False): whether to overwrite an existing dataset of the same
            name

How is this patch tested? If it is not, please explain why.

  • added to DatasetFactoryTests
import fiftyone as fo
datadir = "/path/to/images"
dataset_name = "dataset"

dataset = fo.Dataset.from_dir(
    datadir,
    dataset_type=fo.types.ImageDirectory,
    name=dataset_name,
    persistent=True
)
assert dataset.persistent

# raises ValueError because name "dataset" already taken
fo.Dataset.from_dir(
    datadir,
    dataset_type=fo.types.ImageDirectory,
    name=dataset_name,
    persistent=True
)

# Succeeds
dataset2 = fo.Dataset.from_dir(
    datadir,
    dataset_type=fo.types.ImageDirectory,
    name=dataset_name,
    persistent=True,
    overwrite=True,
)
assert dataset.persistent
assert dataset2.name == dataset_name

What areas of FiftyOne does this PR affect?

  • App: FiftyOne application changes
  • Build: Build and test infrastructure changes
  • Core: Core fiftyone Python library changes
  • Documentation: FiftyOne documentation changes
  • Other

@codecov
Copy link

codecov bot commented May 22, 2023

Codecov Report

Patch coverage has no change and project coverage change: +34.06 🎉

Comparison is base (84d01e7) 15.12% compared to head (1076810) 49.18%.

Additional details and impacted files
@@                 Coverage Diff                  @@
##           release/v0.21.1    #3095       +/-   ##
====================================================
+ Coverage            15.12%   49.18%   +34.06%     
====================================================
  Files                  560      230      -330     
  Lines                68959    34478    -34481     
  Branches               597      325      -272     
====================================================
+ Hits                 10428    16957     +6529     
+ Misses               58531    17521    -41010     
Flag Coverage Δ
app 49.18% <ø> (+34.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 478 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@brimoor brimoor force-pushed the feature/addtl-options-to-fromdir branch from 7be7082 to 781df0a Compare June 12, 2023 01:06
@brimoor brimoor changed the base branch from develop to release/v0.21.1 June 12, 2023 01:06
@brimoor brimoor self-requested a review June 12, 2023 01:06
@brimoor brimoor self-assigned this Jun 12, 2023
@brimoor brimoor added the enhancement Code enhancement label Jun 12, 2023
Copy link
Contributor

@brimoor brimoor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@brimoor brimoor merged commit 397485b into release/v0.21.1 Jun 12, 2023
10 of 11 checks passed
@brimoor brimoor deleted the feature/addtl-options-to-fromdir branch June 12, 2023 02:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Code enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants