-
Notifications
You must be signed in to change notification settings - Fork 122
Optional HTML representations for Jupyter Notebook #743
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
Optional HTML representations for Jupyter Notebook #743
Conversation
Starting point is stac-utils#573 Improvements - Jinja dependency is optional, otherwise __repr__ is used - Catalog shows children - get_items() is kept lazy - Minor improvements in the Jinja templates
- Use macros to simplify Jinja templates - Assets show title before expansion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this @DahnJ! These reprs looks great.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this @DahnJ! I really like the way it's organized; the templates are very understandable, and it's easy to trace a PySTAC class to its template.
Regarding some of the possible "points of improvement" in your description...
- Catalog vs Collection (some fields such as extent are missing from collections since the template simply uses extra_fields)
See my review comments, but I think it makes sense to break these into separate templates. I think that would make it easier to handle some of the extra fields in Collection
(like extent
).
- Additional templates / formatting, e.g. for providers, properties
- Providing some support for extensions (e.g. would be nice to have pretty-formatted eo:bands )
I agree it would be good to flesh out some of the other classes like Provider
and the extension objects.
Should we also add a jinja2
extra here? In general, users would probably be using this functionality in a Jupyter notebook or other environment where jinja2
is already installed, but it might be nice to give them the option to install it as an extra as well.
Tested by building a wheel (python -m build) after a pip install build creating a new virtualenv installing from the built wheel in the new virtualenv installing jupyter notebooks and testing the html repr
Previously, importing pystac would attempt to import jinja This means jinja would be imported even if the html repr was never used
@DahnJ Sorry for the long silence on this, I'm just coming back to this now. I just updated the branch to reflect the latest changes to |
Codecov Report
@@ Coverage Diff @@
## main #743 +/- ##
==========================================
- Coverage 94.64% 94.31% -0.34%
==========================================
Files 81 83 +2
Lines 11803 11873 +70
Branches 1378 1387 +9
==========================================
+ Hits 11171 11198 +27
- Misses 453 496 +43
Partials 179 179
Continue to review full report at Codecov.
|
It looks like we'll also need to configure |
Lazy question: does this handle |
…ptional' into feature/html-repr-optional
jinja2 is used for optional html_repr for Jupyter Notebooks and should thus not be required
…ptional' into feature/html-repr-optional
I will still have a quick look at this tomorrow, but after that this is good to go from my side 👍 |
I have implemented a rudimentary template for This is what it looks like However, I have shown all the items. If I understood it correctly, cc: @duckontheweb |
Great!
I think that's right. We might want to limit the ItemCollection repr to some number of items (maybe 10?). It's common to have an ItemCollection with hundreds or thousands of items. |
Yes, this is correct. These Items will already be resolved in memory.
I agree with @TomAugspurger on this, I think it would be good to limit the number of Items displayed. |
ItemCollection could theoretically have hundreds or thousands of items which shouldn't all be shown in the HTML representation at once
Okay, I pushed 2137ff0 with the changes: If there are But if there are over 10 items, only the first 10 are shown with a helpful message: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for sticking with this one @DahnJ, this is a pretty big feature add to the library!
Related Issue(s): #508
Description
This is a second attempt (after #573) on providing rich HTML representations for (Jupyter) notebooks.
There are still many areas that should/could be improved, but I'm submitting the PR now for feedback. Any feedback is most welcomed, I'd be happy to align this with expectations and requirements from the PySTAC developers/community.
Some areas of possible improvement in this or a subsequent PR
extent
are missing from collections since the template simply usesextra_fields
)eo:bands
)Examples
To reproduce the examples, use the example catalog
Catalog shows children
Collection
Item with Assets
Changes from #573
Based on #573 with the following improvements:
repr
is used.PR Checklist
pre-commit run --all-files
)scripts/test
)