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

new empty pages #3766

Merged
merged 5 commits into from Nov 15, 2023
Merged

new empty pages #3766

merged 5 commits into from Nov 15, 2023

Conversation

imanjra
Copy link
Contributor

@imanjra imanjra commented Nov 2, 2023

What changes are proposed in this pull request?

Add new empty pages for the cases below:

When there are no datasets in FiftyOne

image
image

When there are datasets in FiftyOne, but none is selected

image
image

When a dataset is selected, but there are no samples in the dataset

image
image

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

See preview above

Release Notes

Is this a user-facing change that should be mentioned in the release notes?

  • No. You can skip the rest of this section.
  • Yes. Give a description of this change to be included in the release
    notes for FiftyOne users.

See above

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

@imanjra imanjra marked this pull request as ready for review November 3, 2023 15:26
@imanjra imanjra requested a review from a team November 3, 2023 15:47
@imanjra imanjra changed the base branch from estimated-counts to develop November 3, 2023 15:48
Copy link

codecov bot commented Nov 3, 2023

Codecov Report

Attention: 823 lines in your changes are missing coverage. Please review.

Comparison is base (e0e6505) 15.57% compared to head (0203130) 15.52%.
Report is 13 commits behind head on develop.

Files Patch % Lines
app/packages/core/src/components/Starter/index.tsx 0.00% 243 Missing ⚠️
.../src/plugins/SchemaIO/components/LazyFieldView.tsx 0.00% 92 Missing ⚠️
...pp/packages/core/src/components/Starter/content.ts 0.00% 75 Missing ⚠️
app/packages/operators/src/state.ts 0.00% 70 Missing ⚠️
app/packages/operators/src/hooks.ts 0.00% 69 Missing ⚠️
app/packages/operators/src/loader.tsx 0.00% 54 Missing ⚠️
app/packages/operators/src/types.ts 0.00% 46 Missing ⚠️
...kages/components/src/components/CodeTabs/index.tsx 29.50% 43 Missing ⚠️
app/packages/operators/src/built-in-operators.ts 0.00% 30 Missing ⚠️
app/packages/state/src/recoil/modal.ts 33.33% 20 Missing ⚠️
... and 18 more
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3766      +/-   ##
===========================================
- Coverage    15.57%   15.52%   -0.05%     
===========================================
  Files          645      653       +8     
  Lines        74120    74834     +714     
  Branches       990      996       +6     
===========================================
+ Hits         11542    11620      +78     
- Misses       62578    63214     +636     
Flag Coverage Δ
app 15.52% <8.85%> (-0.05%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

These new pages are looking awesome! Here's some suggested copy tweaks for each page:

No datasets

No datasets yet

# If the `@voxel51/utils/create_dataset` plugin is installed
[Click here](open @voxel51/utils/create_dataset) to create a new dataset, or [browse operations](open operator browser) for other options

# If the `@voxel51/utils/create_dataset` plugin is not installed
Did you know? You can create datasets in the App by installing the [@voxel51/utils](https://github.com/voxel51/fiftyone-plugins/tree/main/plugins/utils) plugin, or [browse operations](open operator browser) for other options

[Learn more](https://docs.voxel51.com/user_guide/dataset_creation/index.html) about loading data into FiftyOne

Create dataset with code

You can use Python to [load data](https://docs.voxel51.com/user_guide/dataset_creation/index.html) into FiftyOne
import fiftyone as fo

# A name for the dataset
name = "my-dataset"

# The directory containing the data to import
dataset_dir = "/path/to/data"

# The type of data being imported
dataset_type = fo.types.COCODetectionDataset

dataset = fo.Dataset.from_dir(
    dataset_dir=dataset_dir,
    dataset_type=dataset_type,
    name=name,
)

No dataset selected

No dataset selected

You can use the selector above to open an existing dataset

# If the `@voxel51/utils/create_dataset` plugin is installed
[Click here](open @voxel51/utils/create_dataset) to create a new dataset, or [browse operations](open operator browser) for other options

# If the `@voxel51/utils/create_dataset` plugin is not installed
Did you know? You can create datasets in the App by installing the [@voxel51/utils](https://github.com/voxel51/fiftyone-plugins/tree/main/plugins/utils) plugin, or [browse operations](open operator browser) for other options

Select a dataset with code

You can use Python to load a dataset in the App
import fiftyone as fo

# Name of an existing dataset
name = "quickstart"

dataset = fo.load_dataset(name)

# Launch a new App session
session = fo.launch_app(dataset)

# If you already have an active App session
# session.dataset = dataset

Dataset with no samples

No samples yet

# If the `@voxel51/io/import_samples` plugin is installed
[Click here](open @voxel51/io/import_samples) to add samples to this dataset, or [browse operations](open operator browser) for other options

# If the `@voxel51/io/import_samples` plugin is not installed
Did you know? You can add samples to datasets in the App by installing the [@voxel51/io](https://github.com/voxel51/fiftyone-plugins/tree/main/plugins/io) plugin, or [browse operations](open operator browser) for other options

[Learn more](https://docs.voxel51.com/user_guide/dataset_creation/index.html#custom-formats) about loading data into FiftyOne

Add samples with code

You can use Python to [add samples](https://docs.voxel51.com/user_guide/dataset_creation/index.html) to this dataset
import fiftyone as fo

dataset = fo.load_dataset("$CURRENT_DATASET_NAME")

samples = []
for filepath, label in zip(filepaths, labels):
    sample = fo.Sample(filepath=filepath)
    sample["ground_truth"] = fo.Classification(label=label)
    samples.append(sample)

dataset.add_samples(samples)

@imanjra imanjra force-pushed the feat/empty-page branch 4 times, most recently from d4ff536 to 34afbfd Compare November 7, 2023 19:42
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 from a copy standpoint! 🥇

@brimoor
Copy link
Contributor

brimoor commented Nov 10, 2023

@imanjra this is super minor, but is it easy to introduce a minimum width for the code blocks? Some of them look a bit narrow (although all code is visible, of course). I'd suggest 80 characters wide since that's our Python style 🤓

It could even be a fixed width layout where anything longer causes horizontal scrolling (which we should make every effect to avoid with our canned code snippets)

Copy link
Contributor

@manivoxel51 manivoxel51 left a comment

Choose a reason for hiding this comment

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

nice work! 🍨

Pulled and tested with an empty dataset.

Thoughts for later: For people checking out FO UI (without the SDK) it might be worth to have a button or operator that can populate the empty dataset with random images/videos so that the user can explore the app with a click of a button

@@ -75,6 +76,8 @@ const DatasetPageQueryNode = graphql`
const DatasetPage: Route<DatasetPageQuery> = ({ prepared }) => {
const data = usePreloadedQuery(DatasetPageQueryNode, prepared);
const isModalActive = Boolean(useRecoilValue(fos.isModalActive));
const count = useRecoilValue(fos.datasetSampleCount);
Copy link
Contributor

Choose a reason for hiding this comment

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

cool, so this is not based on for example filters returning 0 samples but actual sample count of a dataset

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed. This count should not factor in filters applied.

registerOperator(SplitPanel);
registerOperator(SetSelectedLabels);
registerOperator(ClearSelectedLabels);
_registerBuiltInOperator(CopyViewAsJSON);
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at the diff it looks like some built in operators are removed..?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have refactored the operator loading and move some of the loading code out of this file. I have updated registerOperator with _registerBuiltInOperator to bind to built-in plugin. However, I will check to make sure they are not gone from the list 👍

Copy link
Contributor

@benjaminpkane benjaminpkane left a comment

Choose a reason for hiding this comment

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

🚀

@@ -15,6 +15,7 @@ import Nav from "../../components/Nav";
import { Route } from "../../routing";
import style from "../index.module.css";
import { DatasetPageQuery } from "./__generated__/DatasetPageQuery.graphql";
import Starter from "@fiftyone/core/src/components/Starter";
Copy link
Contributor

Choose a reason for hiding this comment

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

Small import nit. It's good to be explicit about what packages the core library exports I think

Suggested change
import Starter from "@fiftyone/core/src/components/Starter";
import { Starter } from "@fiftyone/core";

@imanjra
Copy link
Contributor Author

imanjra commented Nov 14, 2023

Thoughts for later: For people checking out FO UI (without the SDK) it might be worth to have a button or operator that can populate the empty dataset with random images/videos so that the user can explore the app with a click of a button

Good idea. We do have several core operators in https://github.com/voxel51/fiftyone-plugins which allows creating, importing samples, loading zoo datasets and much more right from the app.

@imanjra imanjra merged commit 279935c into develop Nov 15, 2023
12 of 13 checks passed
@imanjra imanjra deleted the feat/empty-page branch November 15, 2023 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants