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 JSON export to Raw visualization #1485

Merged
merged 1 commit into from
Sep 4, 2023
Merged

Add JSON export to Raw visualization #1485

merged 1 commit into from
Sep 4, 2023

Conversation

axelboc
Copy link
Contributor

@axelboc axelboc commented Sep 1, 2023

The Raw visualization now supports exporting datasets to JSON when using the h5grove, h5wasm and mock providers.

This provides a much better solution to issue #917 than PR #1462 . As such, large raw datasets that can't be rendered on the screen are no longer logged to the console.

image

image

Concerning large datasets, I took the opportunity to do some more testing and decided to increase the threshold for when a dataset is considered to be large by a few orders of magnitude. This is purely empirical...

@axelboc axelboc changed the title Add JSON export to raw visualization Add JSON export to Raw visualization Sep 1, 2023
@axelboc axelboc requested review from t20100 and removed request for loichuder September 1, 2023 14:28
Copy link
Member

@t20100 t20100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!
Neater than dumping to the console.

@axelboc axelboc merged commit 3f3f864 into main Sep 4, 2023
8 checks passed
@axelboc axelboc deleted the raw-json branch September 4, 2023 09:37
@@ -8,23 +10,13 @@ function RawVis(props: Props) {
const { value } = props;
const valueAsStr = JSON.stringify(value, null, 2);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This step could be time consuming if value is too big. I wonder if we could infer the size in another lighter way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I did wonder about that. On the other hand, the value first has to be downloaded, which takes a lot more time than JSON.stringify. Even in VS Code we have a 2GB limit on the size of the HDF5 file, so I'm not hugely concerned. Given that the raw vis is for datasets that have complicated dtypes, I'm not sure there's an easier way to approximate the size... So I'd prefer to wait for someone to hit this potential bottleneck before trying to fix it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me 👍

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.

3 participants