SolarArbiter / solarforecastarbiter-core

Core data gathering, validation, processing, and reporting package for the Solar Forecast Arbiter
https://solarforecastarbiter-core.readthedocs.io
MIT License
33 stars 21 forks source link

Fix headers and indexes of report timeseries downloads #722

Closed lboeman closed 2 years ago

lboeman commented 2 years ago

Removes the <br> tags included in csv headers for report timeseries downloads, and separates each timeseries into a separate download. Triggering a bunch of subsequent data downloads is pretty jarring, so perhaps inserting links to download each one individually would be better? We could use something like JSZip to create a zip, but I that is complicated by the need to supply the library in downloaded copies and include it in dashboard rendered reports.

@wholmgren What do you think about adding links to download each timeseries individually?

wholmgren commented 2 years ago

Hmm, that's tough. I see your point, but I also don't want to ask someone to click on 10 different links. How do we create the zips with the html/pdf reports and signature? And could we do both?

lboeman commented 2 years ago

How do we create the zips with the html/pdf reports and signature? And could we do both?

The full report zips are created by making a request to the dashboard: https://github.com/SolarArbiter/solarforecastarbiter-dashboard/blob/40fee97b807ad32185772fce9242869ef96181ac/sfa_dash/blueprints/reports.py#L205-L238

By both do you mean zip up the report and timeseries values together? We could do that, and have a separate endpoint that just zips the values that would be accessible via the current download link.

wholmgren commented 2 years ago

By both do you mean zip up the report and timeseries values together?

No but I guess that's another option. I meant could we allow users to download time series one by one or click a download all button. The download all could result in either 10 downloads or a single zip.

lboeman commented 2 years ago

Here's the next iteration on this. There's a collapsible <details> element that contains a table of timeseries names, and checkboxes. When the user clicks "download csvs" one csv is downloaded per checkbox.
Screenshot from 2021-08-26 15-23-16

wholmgren commented 2 years ago

Nice!

Should be "Download CSVs" (no capitalization on the last s). Or "Download CSV files". We could make it even more clear with something like "Download N CSVs" where N = number of checked boxes and is set by a small javascript function. Or are we overthinking a tiny detail?

lboeman commented 2 years ago

Should be "Download CSVs" (no capitalization on the last s). Or "Download CSV files". We could make it even more clear with something like "Download N CSVs" where N = number of checked boxes and is set by a small javascript function. Or are we overthinking a tiny detail?

I think that's a good idea so I've implemented it and set the "Download CSVs" button to disabled with no download boxes checked, and prompts the user to select CSVs like so:

None checked/ Default: Screenshot from 2021-08-26 15-44-59

Some checked: Screenshot from 2021-08-26 15-45-09

wholmgren commented 2 years ago

And what's new entries please

lboeman commented 2 years ago

Added the what's new, but I realized this will work poorly for distributions, so we should hold off on merging. I'll take a look at it tomorrow morning, I think it won't be too bad.

lboeman commented 2 years ago

Alrighty, Got the distribution downloads working and they look like so:

timestamp,Prob(f <= x) = 2.0%,Prob(f <= x) = 5.0%,Prob(f <= x) = 10.0%,Prob(f <= x) = 20.0%,Prob(f <= x) = 30.0%,Prob(f <= x) = 40.0%,Prob(f <= x) = 50.0%,Prob(f <= x) = 60.0%,Prob(f <= x) = 70.0%,Prob(f <= x) = 80.0%,Prob(f <= x) = 90.0%,Prob(f <= x) = 95.0%,Prob(f <= x) = 98.0%,Prob(f <= x) = 99.0%
2020-08-13T07:00:00-06:00,4.0665,7.9438,13.9053,25.5227,52.2997,61.8447,77.3958,89.075,90.595,93.1887,97.8092,102.633,107.499,109.121