pyresample
pyresample copied to clipboard
Area definition html representation for Jupyter notebooks
This adds _repr_html_ to the AreaDefinition class. In Jupyter notebooks therefore instead of a string a nicely formatted text is show together with a map overview (cartopy plot with borders and coastlines) as can be seen in the example below. The map by default is folded right now to save space (for the screenshot I unfolded it).

The way this is setup is heavily influenced by how xarray implements the _repr_html_. Basically I used the same directory/ file structure. I also copied some code (the html with the svg icons, the function to read the static data) which I did not attribute yet.
While functionally ready some considerations before this get merged:
-
Apart from some refactoring of the plotting function (removing unecessary code and comments) I was thinking about maybe moving the plotting function into the class?
-
Only the
AreaDefinitionhas a html representation as of now. It would be nice if something similar could be done forSwathDefinitionto make the look and feel more homogeneous. -
[ ] Tests added
-
[ ] Tests passed
-
[ ] Passes
git diff origin/main **/*py | flake8 --diff -
[ ] Fully documented
Codecov Report
Attention: 9 lines in your changes are missing coverage. Please review.
Comparison is base (
e82bf47) 94.06% compared to head (5191248) 94.23%. Report is 2 commits behind head on main.
| Files | Patch % | Lines |
|---|---|---|
| pyresample/_formatting_html.py | 94.57% | 7 Missing :warning: |
| pyresample/area_config.py | 92.30% | 1 Missing :warning: |
| pyresample/geometry.py | 83.33% | 1 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #450 +/- ##
==========================================
+ Coverage 94.06% 94.23% +0.17%
==========================================
Files 85 87 +2
Lines 13250 13458 +208
==========================================
+ Hits 12463 12682 +219
+ Misses 787 776 -11
| Flag | Coverage Δ | |
|---|---|---|
| unittests | 94.23% <95.73%> (+0.17%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
coverage: 93.822% (+0.2%) from 93.644% when pulling 5191248f11d2a6c58a357da0b052c9f321b4ffcf on BENR0:feat_html_repr into e82bf470dae00258231978037455ecb2e133e796 on pytroll:main.
You may want to try installing the pre-commit hooks. It looks like you've made the style checkers very angry :wink:
Other than the style issues, what happens if matplotlib and/or cartopy are not installed? These are not hard requirements of pyresample so we should still show something useful if they are not installed.
@djhoese Yeah I already saw it :-D. I think most of the complaints are in the plotting which I need to refactor a little anyway.
Good point about matplotlib/cartopy didn't think about that. But in that case I think it should just fall back to the normal string representation.
I will also Look into the other suggestions you made.
@BENR0 what's the status of this? also, will this affect performance of the notebooks?
Yes I think that should be possible in general. I already did some work in that direction and also towards having a representation for SwathDefinitions. It's not done yet though. If you want to include in a release before PCW I could try and refactor what I have to a satisfactory level to be included. Otherwise working on this as well as on the html representation for Scenes is on my list for PCW :-).
Oh then lets leave this as a task for you for the PCW with the goal of merging it by the end of the week.
Current status:
- html representations for Area as well as SwathDefinition is implemented
- For the time being the SwathDefinition will not get an overview plot due to computing the bounding box lon/lats is currently slow if full resolution lon/lat arrays area used (which for example is the default in modis readers due to interpolation). Tests with only tiepoint arrays suggest that it would be fast enough.
- If xarray or cartopy are not installed html representation is still generated but only for the parts which do not rely on those packages. For the map overview and the lon/lat array display in the SwathDefinition case a note is displayed that further rich display can be enabled when those packages are installed.
- There are no performance implications in Scene loading or dataset display since the html representation is only generated when areas are directly called inside a notebook.
Pre-commit is complaining about missing docstrings:
pyresample/static/__init__.py:1:1: D104 Missing docstring in public package
pyresample/static/html/__init__.py:1:1: D104 Missing docstring in public package
pyresample/static/css/__init__.py:1:1: D104 Missing docstring in public package
Waiting for tests here :)
Ok, so I added some tests for the "cartopy not installed" case. If xarray is not installed I now opted to print the numpy arrays, so there was no need for some extra message for the user. This should work as expected now. There are some things to be noted/ todos:
_repr_html_for theSwathDefinitionhas no Area overview because plotting is to expensive currently in too many cases. This is something I will add again once the G-Ring information is added to theSwathDefinition- There is some improvement possible designwise and codewise for the display of the numpy arrays. I want to refactor that a little later on to make it more similar to the display of the xarray case.
- I am probably going to move the css and the icons inline which will get rid of the extra directories.
- Resolution calculation in the numpy array case is very crude and should be improved/discussed. This might also be something that is superfluous if this is added as metadata/attribute to the
SwathDefinition.
What's the current state of this PR? Should it wait for the next PCW? Or can I plan to include it in the next minor release (1.28.0)?
@djhoese This can be merged. The overview for the SwathDefinition is still missing due to the performance problem. But Martin and I talked about it during PCW and agreed that this is good to go as is and that I will add that at a later point when the problem is solved together with some more refactoring (possibly in conjunction with https://github.com/pytroll/satpy/pull/2167 which should also be good to go and https://github.com/pytroll/satpy/pull/2171).
pre-commit wants a docstring on all of the
__init__.pyfiles in the static directory. Could you add those (3 total)?
Done
@mraspaud @mraspaud I added some more tests. They are pretty rudimentary and just check if the generated html string contains tags specific to the feature being tested.
While working on this I stumbled about the resolution calculation for the SwathDefinition in the numpy case again There are two points: First (as mentioned earlier in the discussion) this is very basic and probably not very precise and therefore should be refactored (ideas welcome, maybe there is something that can be reused already?). Second in the xarray case it is possible that the representation breaks because currently the assumption is from a Satpy view were the Latitude and Longitude DataArrays in SwathDefinitions get assigned attributes like name, resolution and units. So in the case were a user creates a SwathDefinition with xarray lats/lons without those attributes the representation throws an error. These attributes could be made mandatory if SwathDefinitions are created with DataArrays which is a pretty hard constraint I guess or the representation could do more checks and in case these attributes are missing fall back to calculating these as in the numpy case which is "inline" with point one needing some more work. Let me know what you think.
Currently the pure dask.array and numpy case print outs look the same. I think the reason that if dask.arrays are input into the SwathDefinition the printout still shows them as numpy arrays is the following lines https://github.com/pytroll/pyresample/blob/12892b875221846f0fbc7f68775dbdadd8b68584/pyresample/geometry.py#L802-L804. Anyway this is how it looks like (fixed the resolution decimals by rounding, will push it later):
The xarray case looks like this:
The AreaDefinition looks like this (the map portion is folded initially):
What does the SSP mean on the resolution? Single standard parallel?
This looks pretty good to me. In the future we may want to have an optional WKT display and we may want to handle name/ID and description being empty since pyresample 2.0 won't use them by default (it considers them metadata).
SSP = Sub Satellite Point
Yes I thought about the WKT display also. In any case I think there is some room for improvement.
Looks like there are merge conflicts (and somehow appveyor got run again...wtf). Can you fix up the merge conflicts and see how the rest of CI feels about it?
Alright hopefully fixed now.
Sorry for being so late on this. I was a little concerned by how large the coverage loss was. I checked and it looks like the generate_area_def_rst_list function isn't covered. Would it be possible to add a test for it?
I was afraid one of you would notice and ask for a test :-D. I always find it difficult to write tests for something like this but I will have a look at it and come up with something. I am not sure if I will be able to do that before my holiday next week though (I have a long train ride I will try to use but no guarantees).
Where are we at with this?
@djhoese sorry for the delay. I added a test now. I am only checking if area_repr is called because actual checking of the output seems kind of difficult especially due to the plots being included as svg string. The line when an area in the file does not have a _repr_html_ attribute does not get checked. I think this is only the case for stacked and dynamic areas which can not be dumped to yaml anyway right? I can't remember the exact cases why this was needed I can just remember some of the areas throwing errors when I tested this way back. Anyway therefore the test is not really complete I guess.
A couple suggestions and questions, but otherwise coveralls is still saying the SwathDefinition branch of the formatting is not covered. That and the PNG output option. I see mention of swath definitions in your test so any idea why those branches aren't being covered?
I think the SwathDefinition tests just test the html code part and not the overview plot that's why that branch in the plotting function is not covered. I will try and add tests for the plotting function itself or update the test to also cover the SwathDefinition branch. The PNG output option could be removed I just left it there as a feature but it's not used in the html representation. Should I remove it then or add a test (even though I am not sure how write a check for that yet :-D).
If the PNG stuff isn't used in the final output then I guess I'm ok with it being removed. Although...it might be nice to export an area definition to a PNG. A test wouldn't be too bad as long as you just checked that it was saving and maybe check that it isn't all black. I guess it is up to you what is best for the future. Or maybe @mraspaud has an opinion.
The problem with the testing of the plotting function is that I actually don't save the plot to disc but generate the string representation to include it in the html later on. I guess I could check if savefig was called which would be somehow equivalent to checking that something gets saved. I have no idea how to check if the output is all black or not.
/
Given that we're basically depending on another program to output the PNG representation I am OK with just knowing that it isn't empty (0 size).
Ah I see you added specific tests that check savefig.
pre-commit.ci autofix