Add dataset HTML repr #183
Conversation
There was a problem hiding this comment.
@andersy005, this is awesome 🎉
do you anticipate any cons with this proposal to move this tiny application into the orchestrator?
Nope! This will be easy & very welcome. I would propose adding your /xarray/ route as its own router here.
Question: Can the html string returned by this /xarray/ route be embedded in a GitHub comment?
If so, perhaps we'd want to factor the logic that creates this html into a function, so that we can also use it to embed html reprs in the comments provided to users when test runs succeed, e.g. pangeo-forge/staged-recipes#87 (comment).
There was a problem hiding this comment.
I love it so much!
...
I believe this is a good short term solution to ship a much needed feature quickly. This is the same approach used in https://catalog.pangeo.io/.
However, in the long term, I don't think this architecture is optimal for the following reasons:
- Opening the dataset with python on the backend is expensive and ultimately unnecessary, since the Zarr dataset is well described by a compact json file.
- Xarray's html repr is unnecessarily large, with lots of inline custom CSS. That will prevent us from customizing the look and feel of the display.
Instead, I think we should aim to develop a native javascript solution to generating the dataset preview. Ideally an npm package providing react components that we can simply drop in instead of the API call. Maybe something like that already exists? Some inspiration for how this would look can be found at https://idr.github.io/ome-ngff-samples/.
If we go with this solution, we should open a placeholder issue to keep track of that for a future refactor.
I concur... This seemed to be a quick way to get this working from what I discussed with @freeman-lab and @katamartin. I've started working on some draft mockups for a lightweight/long term REPR that takes advantage of information persisted in the Zarr's consolidated metadata.
We could keep track of this in |
| <Alert status='error' align='center' justifyContent='center'> | ||
| <AlertIcon /> | ||
| {`Status Code: ${reprError.status}`} - {reprError.info?.detail} | ||
| </Alert> |
There was a problem hiding this comment.
@katamartin, it turns out that our useSWR's JSONFetcher returns an Error object with a bit of useful information
pangeo-forge.org/src/lib/fetchers.js
Lines 8 to 13 in 8143745
i ended up taking advantage of this and got rid of the /api/repr/xarray API route. So, this change ensures the errors are properly propagated to the client.
Unfortunately, the answer is no... From what I understand, GitHub's markdown can only contain certain HTML elements, and most of what's in Xarray's HTML string isn't supported by GitHub |
Agree with @andersy005 that we can't render Xarray's html but we could post a PNG or the text output from |
|
For GH, let's just stick with the plain-text dataset repr. Perhaps we could check the
|
This PR adds an HTML repr to the dataset section. My approach here consists of an external, lightweight FastAPI application that returns the raw xarray's HTML repr.
http -v 'https://html-reprs.fly.dev/xarray/?url=https://ncsa.osn.xsede.org/Pangeo/pangeo-forge/gpcp-feedstock/gpcp.zarr'I then render this raw HTML inside our existing
DatasetAccordioncomponent: e.g. https://pangeo-forge-j1838fava-pangeo-forge.vercel.app/dashboard/feedstock/42This tiny FastAPI app currently resides here: https://github.com/andersy005/html-reprs. this app can be merged into the
pangeo-forge/pangeo-forge-orchestrator. @cisaacstern, do you anticipate any cons with this proposal to move this tiny application into the orchestrator?