warnings!

[mbostock]

Fixes #536 by introducing a warning mechanism. When a warning is triggered, a ⚠️ icon is displayed in the top-right corner of the plot.

Mousing over the warning indicator gives the tooltip e.g. “1 warning. Please check the console.” The console shows the warnings:

I have implemented three warnings so far:

• When dates are associated with an implicit band scale (typically because of a bar mark)
• When strings that appear to be dates are associated with an implicit band scale
• When strings that appear to be dates are associated with an implicit ordinal or point scale

I imagine we will think of lots of additional warnings in the future, but I think these three cover the most common mistake I’ve seen in Plot. We may want to have another tracking issue to cover ideas for other warnings (e.g., insets resulting in zero-width rects or bars, mixed-sign values with log scales, undefined or invalid data, etc.).

Ref. https://talk.observablehq.com/t/bar-chart-with-dates/6194/3

[Fil]

Excellent!

I've edited the issue number in your comment to #536. I've also uplifted an old console.warn.

Suggestions for follow-ups:

• I wonder if the messages should start with Plot warning: or Observable Plot warning: for easier filtering.
• I'd like to have a top-level option to disable any warning (or use the classic tier system of debug, info, warn, error?)
• how does warn about ignored indices #593 fit into this? We want the user to know that some indices have been ignored, but maybe they now it and want to ignore the warning
• at some point we'll want to expose warn and give access to this system to third party code

[Fil]

Suggested change

	.text("⚠️")
	.attr("cursor", "help")
	.text("⚠️")

semantically this would feel more appropriate than the text cursor, but it occludes the ⚠️ symbol

[mbostock]

I've edited the issue number in your comment to #536.

I was thinking we’d close that with this PR since we’ve implemented the warning system, and then have a new tracking issue for specific warnings. (Or separate issues or sets of related warnings.) But it’s not important. I can live without the joy of closing another issue. 😉

Edit: Oh, whoops, I just realized I previously linked to a totally unrelated issue. Thanks for fixing!

[mbostock]

I wonder if the messages should start with Plot warning: or Observable Plot warning: for easier filtering.

👍

I'd like to have a top-level option to disable any warning (or use the classic tier system of debug, info, warn, error?)

This sounds good, but it’ll be a little tricky to implement because warn may be invoked with very little context (without access to the top-level options), e.g. in maybeShift. But that said, we already assume that all warnings are generated synchronously within a Plot.plot call, so we could set some hidden state that suppresses these warnings temporarily.

how does #593 fit into this? We want the user to know that some indices have been ignored, but maybe they now it and want to ignore the warning

The philosophy I have been working with is that all warnings should be fixable or suppressible: the warnings tell you the recommended fix, but also how to suppress the warning (e.g. by setting the type of the scale explicitly). In this case I guess we’d need a new scale or mark option that indicates you are aware that some of the data is being ignored and you’re okay with it.

This is a reason not to have a top-level option to suppress warnings, since if you suppress at the top-level, you won’t notice if you accidentally introduce a new warning. And the intent of this design is not to encourage people to litter their plot specifications with the equivalent of eslint-ignore-line, but to make their plot specifications less ambiguous so that it’s clear from reading it what they expect to happen. That said, this might be a still pedantic, so I can still see the value in being able to tell Plot to just be quiet.

at some point we'll want to expose warn and give access to this system to third party code

Agreed. Though the current design is predicated on warnings being generated synchronously and this will be brittle if we expose it outside of Plot. We might need to redesign it before exposing, such as by passing an object to marks that implements the warn method.

[mbostock]

I wonder if I can similarly detect when strings are numbers? That would catch errors like using CSV data without typed: true.

[mbostock]

I wonder if the messages should start with Plot warning: or Observable Plot warning: for easier filtering.

For what it’s worth, browser consoles will also filter by file name, so as long as Plot is loaded as e.g. “plot.js”, filtering by “plot” will work even if the messages themselves do not contain the word.

[mbostock]

FWIW I considered adding a warning if you have numeric data with an implicitly ordinal scale (we already have a warning for temporal data with an implicitly ordinal scale). However it seemed like it might introduce some false positives. This will be something to keep an eye on when we release.

I also thought about having the warning include a link to a webpage that gives more detail on how to fix the warning. I think React does this pretty well?