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

fix: handle empty data in column cells gracefully to avoid viz-switching runtime errors #71

Merged
merged 1 commit into from
Jul 14, 2021
Merged

fix: handle empty data in column cells gracefully to avoid viz-switching runtime errors #71

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 2 additions & 10 deletions src/charts/bar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import TooltipContent, { safeDisplayValue } from "../utilities/tooltip-content";
import { numeralFormatting } from "../utilities/utilities";

import * as Dx from "../utilities/types";
import { sortByOrdinalRange } from "./shared";
import { sortByOrdinalRange, getUniqueValues } from "./shared";

interface BarOptions {
selectedDimensions: string[];
Expand Down Expand Up @@ -112,15 +112,7 @@ export const semioticBarChart = (
};
}

const uniqueValues = sortedData.reduce(
(uniques, datapoint) =>
!uniques.find(
(uniqueDimName: string) => uniqueDimName === datapoint[dim1].toString(),
)
? [...uniques, datapoint[dim1].toString()]
: uniques,
[],
);
const uniqueValues = dim1 === "none" ? [] : getUniqueValues(sortedData, dim1);
Copy link
Member Author

Choose a reason for hiding this comment

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

If a user provides a dataset with a column called none, it's going to break other things in the data-explorer because none is already heavily used internally. If we rewrite this in the future, we may want to use a namespaced constant like $$_DX_NONE_$$ for our none column name to reduce the risk of collision with a user's dataframe. For now, I think it's out of scope to change that at the same time.


if (!colorHashOverride && dim1 && dim1 !== "none") {
uniqueValues.forEach((value: string, index: number) => {
Expand Down
21 changes: 20 additions & 1 deletion src/charts/shared.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const sortByOrdinalRange = (
rAccessor: string | (() => void),
secondarySort: string,
data: Dx.DataProps["data"],
): any[] => {
): Dx.DataProps["data"] => {
Copy link
Member Author

Choose a reason for hiding this comment

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

What goes in, comes out again

const subsortData: { [index: string]: SubsortObject } = {};
let subsortArrays: SubsortObject[] = [];
data.forEach((datapoint) => {
Expand Down Expand Up @@ -65,3 +65,22 @@ export const sortByOrdinalRange = (
[],
);
};

/*
Returns uniques values in a column as strings
Safely stringifies different data types, including null/undefined.
*/
export const getUniqueValues = (
points: Dx.DataProps["data"],
accessor: string,
): string[] => {
return [
...new Set(
points.map((d) => {
const value = d[accessor];
// Don't call stringify on a string, as it will add "quote" marks around your value.
return typeof value === "string" ? value : JSON.stringify(value);
}),
),
];
};
10 changes: 2 additions & 8 deletions src/charts/summary.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import HTMLLegend from "../components/HTMLLegend";
import TooltipContent, { safeDisplayValue } from "../utilities/tooltip-content";
import * as Dx from "../utilities/types";
import { numeralFormatting } from "../utilities/utilities";
import { getUniqueValues } from "./shared";

interface SummaryOptions {
chart: Dx.Chart;
Expand Down Expand Up @@ -37,14 +38,7 @@ export const semioticSummaryChart = (

const rAccessor = metric1;

const uniqueValues = data.reduce(
(uniqueArray: string[], datapoint) =>
(!uniqueArray.find(
(dimValue: string) => dimValue === datapoint[dim1].toString(),
) && [...uniqueArray, datapoint[dim1].toString()]) ||
uniqueArray,
[],
);
const uniqueValues = dim1 === "none" ? [] : getUniqueValues(data, dim1);

if (!colorHashOverride && dim1 && dim1 !== "none") {
uniqueValues.sort().forEach((dimValue, index) => {
Expand Down
11 changes: 2 additions & 9 deletions src/charts/xyplot.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import TooltipContent from "../utilities/tooltip-content";
import { numeralFormatting } from "../utilities/utilities";

import * as Dx from "../utilities/types";
import { sortByOrdinalRange } from "./shared";
import { sortByOrdinalRange, getUniqueValues } from "./shared";

import styled from "styled-components";

Expand Down Expand Up @@ -253,14 +253,7 @@ export const semioticXYPlot = (
dim1 &&
dim1 !== "none"
) {
const uniqueValues = sortedData.reduce(
(uniqueArray, datapoint) =>
(!uniqueArray.find(
(uniqueDim: string) => uniqueDim === datapoint[dim1].toString(),
) && [...uniqueArray, datapoint[dim1].toString()]) ||
uniqueArray,
[],
);
const uniqueValues = getUniqueValues(sortedData, dim1);

if (!colorHashOverride) {
uniqueValues.sort().forEach((dimValue: string, index: number) => {
Expand Down