Add CSV export option to zoomed in plots #4252
Conversation
| arr: Array<{ [key: string]: unknown }> | ||
| ) => { | ||
| ensureFileSync(path) | ||
| const csv = await json2csv(arr) |
There was a problem hiding this comment.
We should be able to reuse this package for creating tsv files by adjusting the options.
There was a problem hiding this comment.
Yes! let's please add the second option also. TSV is way simpler to use in a lot of cases.
(I would check if data needs to be sanitized in some way)
extension/src/plots/model/index.ts
Outdated
| } else { | ||
| await writeCsv( | ||
| filePath, | ||
| rawData as unknown as Array<{ [key: string]: unknown }> |
There was a problem hiding this comment.
[Q] How can the data types for the raw data be different depending on the type of export?
There was a problem hiding this comment.
These types were actually just a bandaid so I could test exports. Will adjust the types!
extension/src/plots/model/index.ts
Outdated
| @@ -227,7 +228,11 @@ export class PlotsModel extends ModelWithPersistence { | |||
| return selectedRevisions | |||
| } | |||
|
|
|||
| public savePlotData(plotId: string, filePath: string) { | |||
| public async savePlotData( | |||
There was a problem hiding this comment.
[I] Rather than have a flag argument for savePlotData I would have different functions savePlotDataAsJson, etc. Even if the write functions are passed to a base function.
There was a problem hiding this comment.
that would then give different messages from the webview which could have different telemetry events and tests.
There was a problem hiding this comment.
Took off the flag argument and used separate functions instead!
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
| @@ -344,19 +346,35 @@ export class WebviewMessages { | |||
| return this.plots.getCustomPlots() || null | |||
| } | |||
|
|
|||
| private async exportPlotData(plotId: string) { | |||
| const file = await showSaveDialog(Uri.file('data.json'), { JSON: ['json'] }) | |||
| private async exportPlotDataAsJson(plotId: string) { | |||
There was a problem hiding this comment.
Similar blocks of code found in 2 locations. Consider refactoring.
| this.plots.savePlotData(plotId, file.path) | ||
| void this.plots.savePlotDataAsJson(file.path, plotId) | ||
| } | ||
|
|
There was a problem hiding this comment.
Similar blocks of code found in 2 locations. Consider refactoring.
|
Code Climate has analyzed commit 6e9158d and detected 2 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 98.0% (85% is the threshold). This pull request will bring the total coverage in the repository to 95.1% (0.0% change). View more on Code Climate. |
Demo
Screen.Recording.2023-07-12.at.11.24.55.AM.mov
Part of #4246