Add log scale to y-axis

hughess commented 8 months ago

Description

Adds a log scale to the y-axis of charts by introducing a logScale prop (boolean).

This does not include a log scale for the x-axis.

Charts this has been added to:

Line
Area
Bar
Scatter
Bubble
Mixed Charts

Charts this has not been added to:

Histogram

Possible Additions

Error handling for the following situations:
- Negative values (currently, chart will appear blank when log scale is turned on and negative values are present)
- 100% stacked charts (produces a strange-looking chart)

Example

<LineChart data={growth} x=month y=value title="Normal Scale"/>

<LineChart data={growth} x=month y=value logScale=true title="Log Scale"/>

Major Questions

Should logScale prop be specific to an axis (ylogScale)?
- I lean towards yes if we plan to offer either of the following:
  - x-axis log scale (xLogScale)
  - Secondary y-axis, where either axis or both could have a log scale (y2logScale)
Should this prop be framed as an "axis type" rather than a boolean
- E.g., yType=log, xType=log, y2Type=log

Checklist

[x] For UI or styling changes, I have added a screenshot or gif showing before & after
[x] I have added a changeset

changeset-bot[bot] commented 8 months ago

🦋 Changeset detected

Latest commit: f8619b028afcb9caf530dadc6d09ad97ff244cb7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages

| Name | Type | | ----------------------------- | ----- | | @evidence-dev/core-components | Minor | | @evidence-dev/components | Minor | | @evidence-dev/evidence | Major | | evidence-test-environment | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

vercel[bot] commented 8 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
evidence-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 12, 2023 4:20pm

netlify[bot] commented 8 months ago

Deploy Preview for evidence-development-workspace ready!

Name	Link
Latest commit
Latest deploy log	https://app.netlify.com/sites/evidence-development-workspace/deploys/65283765b12eff5a8fdbbdff
Deploy Preview	https://deploy-preview-1202--evidence-development-workspace.netlify.app
Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

hughess commented 7 months ago

After looking at the secondary y-axis PR again (#874), I think we will need to split this out by axis.

so the prop for now would be “yLogScale”, with “y2LogScale” to be added along with secondary axis

archiewood commented 7 months ago

Definitely want to be able to log independently

archiewood commented 7 months ago

You could probably call it just yLog and y2Log

archiewood commented 7 months ago

thinking: should you be able to optionally control the base of the log here?

like rather than show 1, 10, 100, maybe i want to show 2,4,8,16 etc on the axes. Obviously 10 is the default. but in computing / scientific applications 2 or 8 or e bases are not uncommon

I think echarts has a logBase option.

hughess commented 7 months ago

@archiewood have added your suggestions: prop is now yLog and have added yLogBase which defaults to 10, but can be overridden.

Going to skip the x-axis log scale for now as it's slightly more involved. Will be easy enough to add in the future

archiewood commented 7 months ago

This is perhaps a side note, but certain types of charts seem poorly suited to a log scale.

Notably:

anything showing a notional start from zero (barcharts), since log(0) is undefined
especially anything stacked (eg stacked bar, area), since it's hard to interpret what the values mean, and relative sizes are distorted between series.

Perhaps log scales only really make sense for Line and Scatter Charts. Should be protect people from accidental traps here, or let them do what they want (there might be some valid cases I can't think of).

archiewood commented 7 months ago

This article explains my thoughts better

hughess commented 7 months ago

I agree in general. I think we should not make it available on stacked charts. I’ll add some error handling for that.

I can see a scenario where you’d use it on a single series bar chart - e.g., plotting Covid cases with a bar per day. I think same for a single series area chart.

hughess commented 7 months ago

@archiewood @ItsMeBrianD I think this is ready now - would be great to get a last look at it if you have a chance.

I've added some error handling for multi-series stacked charts, so it'll now display "log axis cannot be used in stacked chart"

archiewood commented 7 months ago

Looking pretty good. All the min max and base stuff works as expected.

Couple of edge cases we should handle:

Negative values / zero values - these should throw

If you don't specify a y, should you be able to yLog? This error needs to be clearer

Is this as expected?

hughess commented 7 months ago

@archiewood thanks.

That makes sense. I think we can easily access the min values in the y columns, so hopefully not to difficult to add that
That's as expected since the y value will be filled in automatically. In this case, there are multiple y columns getting auto-assigned to y, so it's reading that as a multi-series stacked chart and throwing the error.
I'm not sure what's going on here - I can't tell why only one of the series is being displayed

hughess commented 7 months ago

Ok going to call it here. Some edge cases still exist, but I think they are quite small %s.

evidence-dev / evidence