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

More warnings #755

Open
12 tasks
mbostock opened this issue Feb 11, 2022 · 9 comments
Open
12 tasks

More warnings #755

mbostock opened this issue Feb 11, 2022 · 9 comments
Labels
enhancement New feature or request

Comments

@mbostock
Copy link
Member

mbostock commented Feb 11, 2022

Continuing from #536

@mbostock mbostock added the enhancement New feature or request label Feb 11, 2022
@Fil Fil mentioned this issue Feb 17, 2022
@mbostock
Copy link
Member Author

mbostock commented Mar 2, 2022

We might also want a warning for line and area marks when a channel is temporal but not in monotonic order (within a series). But you’ll presumably want a way to disable it in some rare cases and I’m not sure how we’d do that, unless maybe the mark can check if the sort option is null.

@kentr
Copy link
Contributor

kentr commented Mar 7, 2022

Suggestion:

Warning for the cases where mark data is not parallel to facet data. I.e., cases that are warned about here:

When the include or exclude facet mode is chosen, the mark data must be parallel to the facet data: the mark data must have the same length and order as the facet data. If the data are not parallel, then the wrong data may be shown in each facet. The default auto therefore requires strict equality (===) for safety, and using the facet data as mark data is recommended when using the exclude facet mode. (To construct parallel data safely, consider using array.map on the facet data.)

@mkfreeman
Copy link
Contributor

It would be great to surface a warning when someone uses Plot.barY with a continuous x (which results in over-labeling the ticks on the x axis).

Screen Shot 2022-03-16 at 2 39 50 PM

.

Relatedly Plot.barX with a continuous y. Maybe even Plot.groupX with a continuous x and Plot.groupY with a continuous y (nudging them towards binning continuous values)?

@mbostock
Copy link
Member Author

mbostock commented Mar 16, 2022

@mkfreeman Here’s an example of the pattern you describe:

untitled (20)

Plot.barY([[1, 1], [2, 2], [3, 3]], {x: "0", y: "1"}).plot()

The warning would be triggered by values.some(isNumeric) here:

plot/src/scales.js

Lines 150 to 152 in 66142e2

if (values.some(isTemporal)) warn(`Warning: some data associated with the ${key} scale are dates. Dates are typically associated with a "utc" or "time" scale rather than a "${formatScaleType(type)}" scale. If you are using a bar mark, you probably want a rect mark with the interval option instead; if you are using a group transform, you probably want a bin transform instead. If you want to treat this data as ordinal, you can suppress this warning by setting the type of the ${key} scale to "${formatScaleType(type)}".`);
else if (values.some(isTemporalString)) warn(`Warning: some data associated with the ${key} scale are strings that appear to be dates (e.g., YYYY-MM-DD). If these strings represent dates, you should parse them to Date objects. Dates are typically associated with a "utc" or "time" scale rather than a "${formatScaleType(type)}" scale. If you are using a bar mark, you probably want a rect mark with the interval option instead; if you are using a group transform, you probably want a bin transform instead. If you want to treat this data as ordinal, you can suppress this warning by setting the type of the ${key} scale to "${formatScaleType(type)}".`);
else if (values.some(isNumericString)) warn(`Warning: some data associated with the ${key} scale are strings that appear to be numbers. If these strings represent numbers, you should parse or coerce them to numbers. Numbers are typically associated with a "linear" scale rather than a "${formatScaleType(type)}" scale. If you want to treat this data as ordinal, you can suppress this warning by setting the type of the ${key} scale to "${formatScaleType(type)}".`);

I recall considering this warning earlier, and I think I didn’t do it because the default x channel for Plot.barY is indexOf which is numeric. We wouldn’t want the barY shorthand to generate a warning. That said, we could be more selective in our warning:

  • If the channel is numeric and there are any gaps between numbers? Though this would require detecting when the numbers are integers, and presumably would want to support both ascending and descending sequences. Alternatively we could just special-case 0, 1, 2, 3, … but that seems like cheating.
  • If the channel is numeric and there are more than TK distinct values (high cardinality)?
  • Or more generally if there are more than TK distinct values?

Related #74 which is also about high-cardinality ordinal domains. Though that’s not the only problem here—the other reason we want to warn about numeric data being treated as ordinal is that it makes it hard to see missing values (gaps in the data).

@mkfreeman
Copy link
Contributor

Thanks for elaborating @mbostock -- agreed that we don't want the shorthand to generate a warning. I think If the channel is numeric and there are any gaps between numbers? would still miss the common case of too many ticks being drawn (as you noted, #74). I'm not sure if there's a simple way to check if the shorthand is being used (e.g., if the channel was set explicitly or not), but perhaps we could provide a warning if the channel is numeric and it was explicitly declared (using a string or function?).

@mbostock
Copy link
Member Author

perhaps we could provide a warning if the channel is numeric and it was explicitly declared

Generally speaking we try to do the opposite, which is to declare warnings in cases where the Plot specification is ambiguous (underspecified), such as when the scale type is not explicitly declared. This provides a consistent way to suppress warnings by stating more explicitly what you intend to happen (you always add to a specification to suppress a warning, not remove). In other words I don’t think we should give the shorthand a special exemption around this error; if you specify x: (d, i) => i explicitly then I wouldn’t want a warning either.

@mkfreeman
Copy link
Contributor

mkfreeman commented Jul 26, 2022

Just wanted to add to @kentr 's suggestion above that we provide a warning when the arrays aren't strictly equal (which happens if you apply the same filter function to the facet and mark data). Example notebook here: https://observablehq.com/d/a869f84e397d44b7

@Fil
Copy link
Contributor

Fil commented Aug 30, 2022

@yurivish
Copy link
Contributor

It would be nice if there were a warning attached to any plot that has a Plot.geo mark but has not specified a projection, instructing the user to specify projection: null to disable the warning. Right now making a plot with Plot.geo(geojson).plot() produces an empty plot.

I mentioned this on the d3js slack and @Fil added some nuance:

Note that you can use Plot.geo in conjunction with a dot mark (or any other mark that defines x and y scales); in that case the geo will be projected onto the x/y coordinate system. The warning would only be necessary if there is geo and no projection nor x/y scales.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants