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

Automatic list with overviews of inlcuded area definitions for the documentation #2167

Merged
merged 21 commits into from
Jun 13, 2024

Conversation

BENR0
Copy link
Collaborator

@BENR0 BENR0 commented Aug 5, 2022

Based on pytroll/pyresample#450 this generates overviews for all AreaDefinitions included with Satpy and adds them to the resampling section.

I noticed that a quite large number of AreaDefinitions are included which make the section really long now.
Some areas are not included because only AreaDefinitions currently have _repr_html_ (see PR above).

@BENR0 BENR0 requested review from pnuu, djhoese and mraspaud as code owners August 5, 2022 18:18
@codecov
Copy link

codecov bot commented Aug 5, 2022

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.94%. Comparing base (ab55c4a) to head (269315c).
Report is 461 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2167      +/-   ##
==========================================
+ Coverage   95.91%   95.94%   +0.02%     
==========================================
  Files         366      366              
  Lines       53504    53504              
==========================================
+ Hits        51321    51332      +11     
+ Misses       2183     2172      -11     
Flag Coverage Δ
behaviourtests 4.04% <ø> (+<0.01%) ⬆️
unittests 96.03% <ø> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gerritholl
Copy link
Member

This looks interesting. How easy would it be for users to use this feature to generate overviews of areas from their own local YAML files?

Comment on lines 27 to 49
def generate_area_def_list():
"""Create list of available area definitions with overview plot.

Returns:
str
"""
area_list = []

template = ("{area_name}\n"
"{n:->{header_title_length}}\n\n"
".. raw:: html\n\n"
"{content}\n\n"
" <hr>\n\n")

area_file = get_area_file()[0]
for aname in list(_read_yaml_area_file_content(area_file).keys()):
area = get_area_def(aname)
if hasattr(area, "_repr_html_"):
content = "\n".join([x.rjust(len(x) + 5) for x in area_repr(area, include_header=False).split("\n")])
area_list.append(template.format(area_name=aname, n="", header_title_length=len(aname),
content=content))
else:
pass
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this function rather belong to pyresample?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it doesn't really make sense to include in in pyresample because there is no area definition yaml included and this is only useful to generate a rst with a list of area definitions for the documentation.

Copy link
Member

Choose a reason for hiding this comment

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

It is true that pyresample does not have a default yaml file, but it is pyresample that contains all the functions for reading such yaml files. And also, I would argue that for the sake of separation of concerns, it should be included there.
In practice, I wouldn't be against having a default yaml file with a few generic areas in pyresample by the way, that would probably help potential pyresample users in the future...

Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't areas.yaml fit better with pyresample entirely? But pyresample does not have a concept of SATPY_CONFIG_PATH or a config directory search, so it wouldn't know where to find areas.yaml anyway?

Copy link
Member

Choose a reason for hiding this comment

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

Not that anyone asked for my opinion, but I agree with wanting this in pyresample. I'm not a big fan of a default set of areas in pyresample except only as a few examples (like global lon/lat, one LCC, one polar-stereographic, etc)...generic areas. Pyresample is the low-level functionality, but it doesn't know anything about satellite-specific areas or that it is used for "science" at all so to me it is a fine line.

Copy link
Member

Choose a reason for hiding this comment

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

A while ago, I found a small list of generic areas, like europe, north and south america, etc. I'll try to find it again, so we can use that.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

As far as I see, those are just area names without defining anything AreaDefinitiony that we could use.

@BENR0
Copy link
Collaborator Author

BENR0 commented Aug 11, 2022

@gerritholl Curently not that easy I guess. But could easily be refactored to be available to users I think.

@mraspaud
Copy link
Member

mraspaud commented Aug 12, 2022

@gerritholl Curently not that easy I guess. But could easily be refactored to be available to users I think.

Especially if it was in pyresample 😉

@BENR0
Copy link
Collaborator Author

BENR0 commented Aug 12, 2022

@mraspaud @gerritholl ok so where should this go in pyresample? Should I include it in pytroll/pyresample#450 or make a separate PR?

@coveralls
Copy link

coveralls commented Nov 15, 2022

Coverage Status

Coverage increased (+0.2%) to 94.948% when pulling 6edf540 on BENR0:feat_automatic_area_def_overview into 5cc09e0 on pytroll:main.

Copy link
Member

@pnuu pnuu left a comment

Choose a reason for hiding this comment

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

LGTM. A screenshot would be nice :-)

@BENR0
Copy link
Collaborator Author

BENR0 commented Nov 18, 2022

Since this is based on the area html representation is has the same features (properties list/ foldable map).
So this is how it looks like:

Folded

area_definitions_list_fold

Unfolded

area_definitions_list_unfold

@pnuu
Copy link
Member

pnuu commented Nov 23, 2022

There are some problems with building the documentation, but they seem to be because pytroll/pyresample#450 needs to be merged and released first.

@BENR0
Copy link
Collaborator Author

BENR0 commented Nov 28, 2022

There are some problems with building the documentation, but they seem to be because pytroll/pyresample#450 needs to be merged and released first.

Yes correct. Based on some discussions and ideas I had I am working on making this more useable/pretty so please wait with merging this even if pytroll/pyresample#450 gets merged in the meantime.

@BENR0
Copy link
Collaborator Author

BENR0 commented Dec 5, 2022

Since there are quite a few AreaDefinitions in the yaml file I added a searchable table similar to the reader table with links to the definitions so the user does not have to scroll through the whole document. Currently this is still a bit messy since I just included the generation in the conf.py of the documentation for demonstration purposes. If this is wanted I will refactor this. It could be integrated into the generation function I already moved to pyresample but there on its own it would just generated a static table because the searching and sorting depends on the datatable css and js which is integrated in the satpy documentation.

Below you can see how it would look like. One think to note is that all the area definitions also appear in the toc tree which is not a huge deal but not that nice I guess. I think that has something to do with the toc maxdepth setting but I couldn't get that fixed somehow. So if somebody has an idea about that please share.

Apart from that I notice that there are two area definitions I think belong to DynamicAreaDefinitions?:

  • omerc_bb
  • laea_bb
    Right now these produce broken links in the table because obviously there can be no overview generated for those.

area_def_table

@mraspaud mraspaud added the enhancement code enhancements, features, improvements label Feb 2, 2023
@mraspaud
Copy link
Member

mraspaud commented Feb 2, 2023

Just as a reminder for whoever is looking for a status update, this is waiting for completion of pytroll/pyresample#450

@coveralls
Copy link

coveralls commented Jun 5, 2023

Pull Request Test Coverage Report for Build 9483462883

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 96.04%

Totals Coverage Status
Change from base Build 9418285216: 0.0%
Covered Lines: 51561
Relevant Lines: 53687

💛 - Coveralls

@mraspaud
Copy link
Member

pytroll/pyresample#450 is now merged. Do we need a pyresample release before this can work?

@BENR0
Copy link
Collaborator Author

BENR0 commented Dec 13, 2023

I think so because if I see correctly read the docs uses a conda installed pyresample to build the docs.
While the table should build fine in either case it would be nice to have the area names in the yaml file fixed (even if there is no agreement on a file naming pattern yet) #2310.

@gerritholl
Copy link
Member

You can add yourself to AUTHORS.md while you're at it :)

Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

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

I think this can be merged, thanks for the adding it. The filling for water bodies is not always working, but that needs to be fixed in pyresample.

@mraspaud mraspaud merged commit 6ed8698 into pytroll:main Jun 13, 2024
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement code enhancements, features, improvements
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

Add gallery of areas to documentation
6 participants