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

Make _repr_html_ look like _repr_json_ output #1142

Merged
merged 10 commits into from
Jun 6, 2023

Conversation

jsignell
Copy link
Member

@jsignell jsignell commented Jun 5, 2023

Related Issue(s):

Description:

This is similar to #1130 but it adds an html template that looks like the one that jlab uses for _repr_json_. There seemed to be agreement on #1130 that the json repr was a nice repr for stac objects but the lack of support in vscode (and in classic notebook) was a big deterrent.

I mimicked the style of the repr_json output in jupyter lab, but am not depending on any classes since those don't seem to exist in vscode. I think the colors look pretty ok in light and dark mode, but if people disagree there is probably some tweaks we can do.

vscode:
image

jupyter notebook:
image

jupyterlab light:

image

juptyerlab dark:

image

PR Checklist:

  • pre-commit hooks pass locally
  • Tests pass (run scripts/test)
  • Documentation has been updated to reflect changes, if applicable
  • This PR maintains or improves overall codebase code coverage.
  • Changes are added to the CHANGELOG. See the docs for information about adding to the changelog.

@jsignell jsignell linked an issue Jun 5, 2023 that may be closed by this pull request
@jsignell jsignell self-assigned this Jun 5, 2023
@jsignell jsignell requested a review from gadomski June 5, 2023 12:45
@codecov
Copy link

codecov bot commented Jun 5, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.23 🎉

Comparison is base (5402e71) 90.94% compared to head (4f24b04) 91.17%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1142      +/-   ##
==========================================
+ Coverage   90.94%   91.17%   +0.23%     
==========================================
  Files          49       49              
  Lines        6645     6626      -19     
  Branches      982      978       -4     
==========================================
- Hits         6043     6041       -2     
+ Misses        422      410      -12     
+ Partials      180      175       -5     
Impacted Files Coverage Δ
pystac/catalog.py 93.76% <ø> (+0.34%) ⬆️
pystac/collection.py 96.12% <ø> (+0.63%) ⬆️
pystac/html/jinja_env.py 100.00% <ø> (ø)
pystac/item.py 93.08% <ø> (-0.25%) ⬇️
pystac/asset.py 93.50% <100.00%> (+5.19%) ⬆️
pystac/item_collection.py 94.36% <100.00%> (+2.81%) ⬆️
pystac/link.py 91.45% <100.00%> (+1.00%) ⬆️
pystac/provider.py 90.90% <100.00%> (+11.36%) ⬆️
pystac/stac_object.py 94.76% <100.00%> (+0.20%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@jsignell
Copy link
Member Author

jsignell commented Jun 5, 2023

There is a minor issue with indentation and then I need to add a few tests, so I'm switching this to WIP

@jsignell jsignell marked this pull request as draft June 5, 2023 13:29
@jsignell
Copy link
Member Author

jsignell commented Jun 5, 2023

Ok here is some deeper nesting after a0eaf55
Screenshot from 2023-06-05 10-20-31
Screenshot from 2023-06-05 10-19-59

@jsignell jsignell marked this pull request as ready for review June 5, 2023 15:04
@jsignell jsignell mentioned this pull request Jun 5, 2023
5 tasks
Copy link
Member

@gadomski gadomski left a comment

Choose a reason for hiding this comment

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

I like this a lot, and added bonus that it's a code-removing PR 🎉!

pystac/stac_object.py Outdated Show resolved Hide resolved
@jsignell jsignell requested a review from gadomski June 6, 2023 14:00
@jsignell jsignell added this pull request to the merge queue Jun 6, 2023
@jsignell jsignell removed this pull request from the merge queue due to a manual request Jun 6, 2023
@jsignell
Copy link
Member Author

jsignell commented Jun 6, 2023

I just noticed that the notebook size got kind of big with this change. Taking a look now to see if I can improve that experience

@jsignell
Copy link
Member Author

jsignell commented Jun 6, 2023

Ok I did some mild simplification. The sections are still expanded by default when rendered in github, but they are in the old repr as well. And they both look fine on nbviewer: https://nbviewer.org/github/jsignell/pystac/blob/html-like-json/docs/quickstart.ipynb

MAIN:
image

THIS PR:
image

@jsignell jsignell added this pull request to the merge queue Jun 6, 2023
Merged via the queue into stac-utils:main with commit 334c4c4 Jun 6, 2023
@jsignell jsignell deleted the html-like-json branch June 6, 2023 16:44
@gadomski gadomski added this to the 1.8 milestone Jun 7, 2023
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.

HTML rendering should show asset keys _repr_html_ causes GETs
2 participants