observablehq / plot

A concise API for exploratory data visualization implementing a layered grammar of graphics
https://observablehq.com/plot/
ISC License
4.41k stars 176 forks source link

More warnings #755

Open mbostock opened 2 years ago

mbostock commented 2 years ago

Continuing from #536…

mbostock commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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:

https://github.com/observablehq/plot/blob/66142e2c096439cffea0ed390de13c6cf1545388/src/scales.js#L150-L152

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:

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago
yurivish commented 1 year ago

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.