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

Merge release/v1.1.0 to develop #5154

Merged
merged 11 commits into from
Nov 19, 2024
Merged

Merge release/v1.1.0 to develop #5154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
828ec5c
Fiftyone to FiftyOne
imanjra Nov 19, 2024
597b32f
fixes and enhancements for model evaluation panel
imanjra Nov 19, 2024
01ea636
fixes for embeddings panel
imanjra Nov 19, 2024
ebf8784
Merge branch 'release/v1.1.0' of https://github.com/voxel51/fiftyone …
voxel51-bot Nov 19, 2024
90c4838
prevent video time from becoming NaN when frame rate is undefined
tom-vx51 Nov 19, 2024
45c82b1
fix download links (#5153)
benjaminpkane Nov 19, 2024
3246d80
Merge branch 'release/v1.1.0' of https://github.com/voxel51/fiftyone …
voxel51-bot Nov 19, 2024
d9b2a81
Add support for cell tooltip in TableView (#5152)
minhtuev Nov 19, 2024
8593a1a
Merge branch 'release/v1.1.0' of https://github.com/voxel51/fiftyone …
voxel51-bot Nov 19, 2024
9967d21
Merge pull request #5155 from voxel51/bugfix/prevent-nan-video-time
tom-vx51 Nov 19, 2024
29c03fa
Merge branch 'release/v1.1.0' of https://github.com/voxel51/fiftyone …
voxel51-bot Nov 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions app/packages/components/src/components/PanelCTA/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ export default function PanelCTA(props: PanelCTAProps) {
mode,
name,
onBack,
tryLink,
demoDocCaption,
} = props;
const theme = useTheme();
const isDefault = mode === "default";
Expand All @@ -43,6 +45,9 @@ export default function PanelCTA(props: PanelCTAProps) {
const computedCaption = IS_APP_MODE_FIFTYONE
? demoCaption || caption
: caption;
const computedDocCaption = IS_APP_MODE_FIFTYONE
? demoDocCaption || docCaption
: docCaption;

return (
<Stack spacing={1} sx={{ height: "100%", p: 2 }}>
Expand Down Expand Up @@ -114,7 +119,7 @@ export default function PanelCTA(props: PanelCTAProps) {
<MuiButton
variant="contained"
color="primary"
href={TRY_IN_BROWSER_LINK}
href={tryLink || TRY_IN_BROWSER_LINK}
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add security attributes for external link.

When using target="_blank", it's recommended to add rel="noopener noreferrer" to prevent potential security vulnerabilities:

-href={tryLink || TRY_IN_BROWSER_LINK}
-target="_blank"
+href={tryLink || TRY_IN_BROWSER_LINK}
+target="_blank"
+rel="noopener noreferrer"

Committable suggestion skipped: line range outside the PR's diff.

target="_blank"
>
Try in browser
Expand All @@ -127,9 +132,9 @@ export default function PanelCTA(props: PanelCTAProps) {
spacing={1}
sx={{ alignItems: "center", justifyContent: "center" }}
>
{docCaption && (
{computedDocCaption && (
<Typography color="secondary">
{docCaption || "Not ready to upgrade yet?"}
{computedDocCaption}
</Typography>
)}
<MuiButton
Expand Down Expand Up @@ -179,4 +184,6 @@ export type PanelCTAProps = {
demoLabel?: string;
demoDescription?: string;
demoCaption?: string;
demoDocCaption?: string;
tryLink?: string;
};
2 changes: 1 addition & 1 deletion app/packages/core/src/components/Starter/content.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export const CONTENT_BY_MODE = {
codeSubtitle:
"Use Python or command line tools to set dataset for the current session",
learnMoreLink: "https://docs.voxel51.com/user_guide/app.html",
learnMoreLabel: "about using the Fiftyone App",
learnMoreLabel: "about using the FiftyOne App",
},
ADD_SAMPLE: {
title: "No samples yet",
Expand Down
38 changes: 32 additions & 6 deletions app/packages/core/src/plugins/SchemaIO/components/NativeModelEvaluationView/Evaluation.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ import { formatValue, getNumericDifference, useTriggerEvent } from "./utils";

const KEY_COLOR = "#ff6d04";
const COMPARE_KEY_COLOR = "#03a9f4";
const DEFAULT_BAR_CONFIG = { sortBy: "az" };
const DEFAULT_BAR_CONFIG = { sortBy: "default" };

export default function Evaluation(props: EvaluationProps) {
const {
Expand Down Expand Up @@ -164,11 +164,15 @@ export default function Evaluation(props: EvaluationProps) {
const evaluationTimestamp = evaluationInfo.timestamp;
const evaluationConfig = evaluationInfo.config;
const evaluationMetrics = evaluation.metrics;
const evaluationType = evaluationConfig.type;
const compareEvaluationInfo = compareEvaluation?.info || {};
const compareEvaluationKey = compareEvaluationInfo?.key;
const compareEvaluationTimestamp = compareEvaluationInfo?.timestamp;
const compareEvaluationConfig = compareEvaluationInfo?.config || {};
const compareEvaluationMetrics = compareEvaluation?.metrics || {};
const compareEvaluationType = compareEvaluationConfig.type;
const isObjectDetection = evaluationType === "detection";
const isSegmentation = evaluationType === "segmentation";
const infoRows = [
{
id: "evaluation_key",
Expand All @@ -179,8 +183,8 @@ export default function Evaluation(props: EvaluationProps) {
{
id: "type",
property: "Type",
value: evaluationConfig.type,
compareValue: compareEvaluationConfig.type,
value: evaluationType,
compareValue: compareEvaluationType,
},
{
id: "method",
Expand Down Expand Up @@ -322,6 +326,7 @@ export default function Evaluation(props: EvaluationProps) {
property: "Average Confidence",
value: evaluationMetrics.average_confidence,
compareValue: compareEvaluationMetrics.average_confidence,
hide: isSegmentation,
},
{
id: "support",
Expand All @@ -340,6 +345,7 @@ export default function Evaluation(props: EvaluationProps) {
property: "IoU Threshold",
value: evaluationConfig.iou,
compareValue: compareEvaluationConfig.iou,
hide: !isObjectDetection,
},
{
id: "precision",
Expand All @@ -364,6 +370,7 @@ export default function Evaluation(props: EvaluationProps) {
property: "mAP",
value: evaluationMetrics.mAP,
compareValue: compareEvaluationMetrics.mAP,
hide: !isObjectDetection,
},
{
id: "tp",
Expand All @@ -377,6 +384,7 @@ export default function Evaluation(props: EvaluationProps) {
? "compare"
: "selected"
: false,
hide: !isObjectDetection,
},
{
id: "fp",
Expand All @@ -391,6 +399,7 @@ export default function Evaluation(props: EvaluationProps) {
? "compare"
: "selected"
: false,
hide: !isObjectDetection,
},
{
id: "fn",
Expand All @@ -405,6 +414,7 @@ export default function Evaluation(props: EvaluationProps) {
? "compare"
: "selected"
: false,
hide: !isObjectDetection,
},
];

Expand Down Expand Up @@ -630,7 +640,9 @@ export default function Evaluation(props: EvaluationProps) {
filterable,
id: rowId,
active,
hide,
} = row;
if (hide) return null;
const difference = getNumericDifference(
value,
compareValue
Expand Down Expand Up @@ -670,7 +682,13 @@ export default function Evaluation(props: EvaluationProps) {
alignItems: "center",
}}
>
<Typography>{formatValue(value)}</Typography>
<Typography>
{value ? (
formatValue(value)
) : (
<Typography color="text.tertiary">—</Typography>
)}
</Typography>
<Stack direction="row" spacing={1}>
{showTrophy && (
<Typography sx={{ fontSize: 12 }}>
Expand Down Expand Up @@ -706,7 +724,13 @@ export default function Evaluation(props: EvaluationProps) {
sx={{ justifyContent: "space-between" }}
>
<Typography>
{formatValue(compareValue)}
{compareValue ? (
formatValue(compareValue)
) : (
<Typography color="text.tertiary">
—
</Typography>
)}
</Typography>

{filterable && (
Expand Down Expand Up @@ -1516,7 +1540,7 @@ function getMatrix(matrices, config) {

function getConfigLabel({ config, type, dashed }) {
const { sortBy } = config;
if (!sortBy) return "";
if (!sortBy || sortBy === DEFAULT_BAR_CONFIG.sortBy) return "";
const sortByLabels =
type === "classPerformance"
? CLASS_PERFORMANCE_SORT_OPTIONS
Expand Down Expand Up @@ -1558,12 +1582,14 @@ function useActiveFilter(evaluation, compareEvaluation) {
}

const CLASS_PERFORMANCE_SORT_OPTIONS = [
{ value: "default", label: "Default" },
{ value: "az", label: "Alphabetical (A-Z)" },
{ value: "za", label: "Alphabetical (Z-A)" },
{ value: "best", label: "Best performing" },
{ value: "worst", label: "Worst performing" },
];
const CONFUSION_MATRIX_SORT_OPTIONS = [
{ value: "default", label: "Default" },
{ value: "az", label: "Alphabetical (A-Z)" },
{ value: "za", label: "Alphabetical (Z-A)" },
{ value: "mc", label: "Most common classes" },
Expand Down
1 change: 1 addition & 0 deletions ...ackages/core/src/plugins/SchemaIO/components/NativeModelEvaluationView/EvaluationPlot.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export default function EvaluationPlot(props: EvaluationPlotProps) {
color: theme.text.secondary,
gridcolor: theme.primary.softBorder,
automargin: true, // Enable automatic margin adjustment
scaleanchor: "x",
},
autosize: true,
margin: { t: 20, l: 50, b: 50, r: 20, pad: 0 },
Expand Down
22 changes: 20 additions & 2 deletions app/packages/core/src/plugins/SchemaIO/components/NativeModelEvaluationView/Overview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
import { LoadingDots } from "@fiftyone/components";
import { Card, CardActionArea, Chip, Stack, Typography } from "@mui/material";
import {
Card,
CardActionArea,
Chip,
Stack,
Typography,
useTheme,
} from "@mui/material";
import React from "react";
import Evaluate from "./Evaluate";
import EvaluationNotes from "./EvaluationNotes";
Expand Down Expand Up @@ -64,6 +71,8 @@ export default function Overview(props: OverviewProps) {

function EvaluationCard(props: EvaluationCardProps) {
const { pending, onSelect, eval_key, note, status, id } = props;
const theme = useTheme();

return (
<CardActionArea key={eval_key} disabled={pending}>
<Card
Expand All @@ -77,7 +86,16 @@ function EvaluationCard(props: EvaluationCardProps) {
{eval_key}
</Typography>
{pending && (
<Chip variant="filled" label={<LoadingDots text="Evaluating" />} />
<Chip
variant="filled"
size="small"
label={
<LoadingDots
text="Evaluating"
style={{ fontSize: "1rem", paddingLeft: 6, color: "#999999" }}
/>
}
/>
)}
{status && <Status status={status} />}
</Stack>
Expand Down
15 changes: 9 additions & 6 deletions app/packages/core/src/plugins/SchemaIO/components/NativeModelEvaluationView/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { PanelCTA } from "@fiftyone/components";
import { constants } from "@fiftyone/utilities";
import { Box } from "@mui/material";
import React, { useCallback, useMemo } from "react";
import EmptyOverview from "./EmptyOverview";
import Evaluate from "./Evaluate";
import Evaluation from "./Evaluation";
import Overview from "./Overview";
import { useTriggerEvent } from "./utils";
import { PanelCTA } from "@fiftyone/components";
import { constants } from "@fiftyone/utilities";
import Evaluate from "./Evaluate";

const TRY_LINK = "http://voxel51.com/try-evaluation";
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Move URL constant to centralized configuration and ensure URL consistency

The codebase shows inconsistent URL patterns that should be standardized:

  • Similar URLs are defined in multiple locations with different formats:
    • app/packages/utilities/src/constants.ts: Uses https:// and consistent domain format
    • app/packages/embeddings/src/EmbeddingsCTA.tsx: Uses http:// with different URL pattern
    • Current file: Uses http:// with different URL pattern

Recommendations:

  • Move the URL to app/packages/utilities/src/constants.ts alongside other Voxel51 URLs
  • Use HTTPS protocol consistently
  • Follow the URL pattern established in TRY_IN_BROWSER_LINK
🔗 Analysis chain

Consider moving the URL constant to a centralized configuration.

The hardcoded URL should be moved to a centralized configuration file for better maintainability and consistency.

Let's verify if similar URLs are defined elsewhere:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for similar URL patterns across the codebase
rg -i "voxel51\.com/try-" --type ts --type tsx

Length of output: 76

Script:

#!/bin/bash
# Search for similar URL patterns across the codebase with correct file extensions
rg -i "voxel51\.com/try" -t ts -t typescript -t jsx -t tsx

# Search for any configuration files that might contain URLs
fd -e ts -e tsx -e json "config|constants" --exec grep -l "voxel51.com"

# Look for any centralized URL or link configurations
ast-grep --pattern 'const $_ = "https?://voxel51.com/$_"'

Length of output: 269

Script:

#!/bin/bash
# Let's check the content of the constants file that was found
cat ./app/packages/utilities/src/constants.ts

# Also search with ripgrep without file type restrictions
rg -i "voxel51\.com/try"

Length of output: 3109


export default function NativeModelEvaluationView(props) {
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add TypeScript type annotations for better type safety.

The component and its props lack TypeScript type annotations. Consider adding proper type definitions to improve type safety and developer experience.

Example improvement:

interface NativeModelEvaluationViewProps {
  data?: {
    evaluations?: Array<{ key: string; id: string }>;
    view?: Record<string, unknown>;
    statuses?: Record<string, unknown>;
    notes?: Record<string, unknown>;
    permissions?: Record<string, unknown>;
    pending_evaluations?: Array<unknown>;
  };
  schema: {
    view: {
      on_change_view: string;
      on_evaluate_model: string;
      load_evaluation: string;
      load_evaluation_view: string;
      set_status: string;
      set_note: string;
      load_view: string;
    };
  };
  onChange: (path: string, value: unknown) => void;
  layout: unknown;
}

export default function NativeModelEvaluationView(props: NativeModelEvaluationViewProps) {
  // ... rest of the component
}

const { data = {}, schema, onChange, layout } = props;
Expand Down Expand Up @@ -87,10 +88,11 @@ export default function NativeModelEvaluationView(props) {
(showEmptyOverview || showCTA ? (
<PanelCTA
label="Create you first model evaluation"
demoLabel="Upgrade to Fiftyone Teams to Evaluate Models"
demoLabel="Upgrade to FiftyOne Teams to Evaluate Models"
description="Analyze and improve models collaboratively with your team"
docLink="https://docs.voxel51.com/user_guide/evaluation.html"
docCaption="Not ready to upgrade yet? Learn how to create model evaluation via code."
docCaption="Learn how to create model evaluation via code."
demoDocCaption="Not ready to upgrade yet? Learn how to create model evaluation via code."
icon="ssid_chart"
Actions={() => {
return (
Expand All @@ -106,6 +108,7 @@ export default function NativeModelEvaluationView(props) {
setShowCTA(false);
}}
mode={showCTA ? "default" : "onboarding"}
tryLink={TRY_LINK}
/>
) : (
<Overview
Expand Down
31 changes: 26 additions & 5 deletions app/packages/core/src/plugins/SchemaIO/components/TableView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
TableContainer,
TableHead,
TableRow,
Tooltip,
useTheme,
} from "@mui/material";
import { isPlainObject } from "lodash";
Expand All @@ -34,6 +35,7 @@ export default function TableView(props: ViewPropsType) {
size = "small",
variant = "filled",
max_inline_actions = 1,
tooltips = []
} = view;
const { rows, selectedCells, selectedRows, selectedColumns } =
getTableData(props);
Expand Down Expand Up @@ -63,6 +65,14 @@ export default function TableView(props: ViewPropsType) {
return computedRowActions;
}, []);

const getTooltips = useCallback((tooltipList) => {
const tooltipDict = {};
for (const { value, row, column } of tooltipList) {
tooltipDict[`${row},${column}`] = value; // Create a key from row and column
}
return tooltipDict;
}, []);
Comment on lines +68 to +74
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance type safety and memoization of getTooltips

The function implementation could be improved in several ways:

  1. Add dependency array to useCallback to prevent unnecessary recreations
  2. Add TypeScript type annotations
  3. Use a more type-safe approach for coordinate keys

Consider this implementation:

- const getTooltips = useCallback((tooltipList) => {
+ type TooltipItem = { value: string; row: number; column: number };
+ type TooltipMap = { [key: string]: string };
+ 
+ const getTooltips = useCallback((tooltipList: TooltipItem[]): TooltipMap => {
    const tooltipDict = {};
    for (const { value, row, column } of tooltipList) {
-     tooltipDict[`${row},${column}`] = value; // Create a key from row and column
+     tooltipDict[`${row},${column}`] = value;
    }
    return tooltipDict;
- }, []);
+ }, [tooltipList]);

Committable suggestion skipped: line range outside the PR's diff.


const handleCellClick = useCallback(
(row, column) => {
if (on_click_cell) {
Expand Down Expand Up @@ -93,6 +103,7 @@ export default function TableView(props: ViewPropsType) {
color: theme.palette.text.secondary,
};
const filled = variant === "filled";
const tooltipMap = getTooltips(tooltips);

return (
<Box {...getComponentProps(props, "container")}>
Expand Down Expand Up @@ -170,23 +181,33 @@ export default function TableView(props: ViewPropsType) {
selectedCells.has(coordinate) ||
isRowSelected ||
selectedColumns.has(columnIndex);
return (

const tooltip = tooltipMap[coordinate]; // Check if there's a tooltip for the cell

const cell = (
<TableCell
key={key}
sx={{
background: isSelected
? selectedCellColor
: "unset",
background: isSelected ? selectedCellColor : "unset",
}}
onClick={() => {
handleCellClick(rowIndex, columnIndex);
}}
{...getComponentProps(props, "tableBodyCell")}
>
{formatCellValue(item[key], props)}
{tooltip ? (
<Tooltip title={tooltip} arrow>
<span>{formatCellValue(item[key], props)}</span> {/* Wrap content with Tooltip */}
</Tooltip>
) : (
formatCellValue(item[key], props) // No Tooltip for cells without tooltips
)}
</TableCell>
);

return cell;
})}

{hasRowActions && (
<TableCell
align="right"
Expand Down
2 changes: 1 addition & 1 deletion app/packages/embeddings/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
# Embeddings Fiftyone Plugin Example
# Embeddings FiftyOne Plugin Example
Loading
Loading