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

add separate download data option to charts #531

Merged
merged 4 commits into from Dec 15, 2022

Conversation

yukseltron
Copy link
Contributor

Description

Resolves #528

Hopefully, this is what was requested.

  • DownloadData now has text as prop ("Download Data", "Save Image", etc). Might be good to rename (DownloadButton)
  • Data added as prop to ECharts component
  • Echarts now has download option for Chart PNG and CSV data

Before:

Screen Shot 2022-12-14 at 7 47 36 PM

After:

recording.mp4

@changeset-bot
Copy link

changeset-bot bot commented Dec 15, 2022

🦋 Changeset detected

Latest commit: e1a19e8

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

This PR includes changesets to release 3 packages
Name Type
@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
Copy link

vercel bot commented Dec 15, 2022

@yukseltron is attempting to deploy a commit to the Evidence Dev Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Dec 15, 2022

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

Name Status Preview Updated
evidence-development-workspace ✅ Ready (Inspect) Visit Preview Dec 15, 2022 at 10:05PM (UTC)
evidence-docs ✅ Ready (Inspect) Visit Preview Dec 15, 2022 at 10:05PM (UTC)

@archiewood
Copy link
Member

archiewood commented Dec 15, 2022

Thanks for contributing on this!

There are actually some relatively specific customer requirements here which I didn't put into the issue, apologies:

  1. The data needs to be just the columns used in the chart, not the entire contents of the query. This is because in the customer project they create multiple charts from a single query.

  2. The data needs to retain the "shape", ie pivots it has in the graph, (if relevant).

  • For example if you had a <BarChart data={myquery} x=year y=sales series=category/> from a query that has columns: year, country, category, sales, the download would have data structure:
Year Category A Category B
2010 $10,000 $20,000
2011 $11,000 $21,000
  • Notice the sales column is pivoted by the series column: any time a series column is present, it pivots the y column.
  1. The download format needs to be csv. We use a package for this in the DownloadData for QueryViewer

Edit: Ignore #3, I see it's already csv!

@mcrascal
Copy link
Member

@archiewood is maintaining the shape of the data a hard requirement?

As a user, I would have a strong preference for the "long" output (as it is now), which is suitable as an input for excel pivot table and/or notebook tool.

@archiewood
Copy link
Member

Fair point. I'm not 100% sure but I guess it would depend on the background of the user.

I think "data people" would typically prefer "long" data, but would guess that most other users would expect it pre pivoted?

@archiewood
Copy link
Member

archiewood commented Dec 15, 2022

UI

In terms of the UI, I think this works well.

Data

In terms of the data returned in the csv, I worry this is not predictable enough for users. ~

Examples:

  1. Two charts that contain the same chart type return data in different formats:
  2. Data includes columns that are not shown in the charts, but are in the query

I think we might need a way to make the data a more predictable structure for users. Not fully sure on the approach yet given the data queries return can be in different shapes.

Rough ideas on an approaches:

  1. Start from the {data} object returned from a query

    • Get the data object from the query
    • Filter to only grab columns shown in chart. This might be tricky as sometimes columns are specified in params, and sometimes they are inferred.
    • (Possibly) do some data transformations to make the data always "long" (one record per row), or always wide (multiple columns with data, when multi series)
  2. Start from the data passed to the chart

    • Not sure what this looks like, but logically the transformations and filtering must have already been applied!
    • But possible that the structure is unhelpful!

EDIT:

We've just had a bit of a discussion internally and come to the view that the approach in the PR now works pretty well. On the above examples:

  1. The differences between returned data on the two different charts just reflect differences between the queries themselves. In both cases you are getting back a tidy data set with one row per observation, and one column per measurement. Fundamentally, they are in the same format, not different formats.
  2. If users really want just a specific set of columns to download, they can make a last-mile query which just selects the columns they want from the larger query, and pass that to the component.
    • It would be good to restrict the columns to those displayed, but that it a pretty gnarly bit of data handling, and could involve touching basically every chart components

@archiewood archiewood merged commit 5ec1961 into evidence-dev:main Dec 15, 2022
@archiewood
Copy link
Member

Once again TY @yukseltron!

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

Successfully merging this pull request may close these issues.

Download data from charts
3 participants