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

Extend aeroval to plot satellite pixels maps #1347

Merged
merged 31 commits into from
Sep 30, 2024

Conversation

lewisblake
Copy link
Member

@lewisblake lewisblake commented Sep 24, 2024

Change Summary

  • Remove old json file computations (in contour directory)
  • ranges.json contains a unit for each variable
  • Implement requested attributes in ModelMapsSetup, and associated helper classes and methods, such as BoundingBox and validate_plot_types
  • Implement plot_overlay_pixel_maps
  • Rename _process_map_var to _process_contour_map_var
  • Fix tests

I have added some small tests for modelmaps_opts in setup_classes.py but not for the actual processing of the pixel maps as analogous tests do not seem to exist for the contour plotting.

Related issue number

closes #401, https://github.com/metno/AeroToolsIssues/issues/5

Checklist

  • Start with a draft-PR
  • The PR title is a good summary of the changes
  • PR is set to AeroTools and a tentative milestone
  • Documentation reflects the changes where applicable
  • Tests for the changes exist where applicable
  • Tests pass locally
  • Tests pass on CI
  • At least 1 reviewer is selected
  • Make PR ready to review

@lewisblake lewisblake added this to the m2024-10 milestone Sep 24, 2024
@lewisblake lewisblake added new-feature SESAM Issues related to the SESAM project labels Sep 24, 2024
Copy link

codecov bot commented Sep 27, 2024

Codecov Report

Attention: Patch coverage is 56.14035% with 25 lines in your changes missing coverage. Please review.

Project coverage is 78.68%. Comparing base (931e6ad) to head (d40203e).
Report is 39 commits behind head on main-dev.

Files with missing lines Patch % Lines
pyaerocom/aeroval/setup_classes.py 38.09% 13 Missing ⚠️
pyaerocom/aeroval/modelmaps_engine.py 36.36% 7 Missing ⚠️
pyaerocom/aeroval/_processing_base.py 25.00% 3 Missing ⚠️
pyaerocom/aeroval/experiment_output.py 88.88% 1 Missing ⚠️
pyaerocom/aeroval/experiment_processor.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           main-dev    #1347      +/-   ##
============================================
- Coverage     78.97%   78.68%   -0.29%     
============================================
  Files           136      136              
  Lines         20817    20787      -30     
============================================
- Hits          16440    16357      -83     
- Misses         4377     4430      +53     
Flag Coverage Δ
unittests 78.68% <56.14%> (-0.29%) ⬇️

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.

@lewisblake lewisblake marked this pull request as ready for review September 27, 2024 12:45
Copy link
Member

@heikoklein heikoklein left a comment

Choose a reason for hiding this comment

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

Looks good, with a few comments.

Use assert only for debugging assumptions, not as exception shortcut.

Discuss with Thorbjørn if image-types should be stored as attribute of the blob.

return [fp_json, fp_geojson]
with self.avdb.lock():
self.avdb.put_map_overlay(
overlay_plot,
Copy link
Member

Choose a reason for hiding this comment

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

Does this mean that put_map_overlay has no knowledge of the overlay_plot image-type (png/jpeg,webp)? And the get_map_overlay does not have that knowledge either?

@thorbjoernl
I would feel better if the image-type were known..., e.g.
ROUTE_MAP_OVERLAY = "/v0/map-overlay/{project}/{experiment}/{source}/{variable}/{date}.{img_type}" would even indicate that the files on dict in the jsonfiledb are images...

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes I think this could make sense as well. I will not stop this from merging as you have approved the PR, but perhaps something to pick up later.

fig, axis = plt.subplots(
1,
1,
subplot_kw=dict(projection=ccrs.Mercator()),
Copy link
Member

Choose a reason for hiding this comment

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

Why are there two projections, Mercator and PlateCarree in this function? We should only have one output projection.

Copy link
Member Author

Choose a reason for hiding this comment

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

The transform argument tells cartopy which projection our data is in. The projection argument tells cartopy which projection to project into. https://scitools.org.uk/cartopy/docs/latest/tutorials/understanding_transform.html

for m in v:
if not isinstance(v[m], set):
v[m] = set([v[m]])
assert v[m] in PLOT_TYPE_OPTIONS
Copy link
Member

Choose a reason for hiding this comment

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

The python compiler may remove all assert statements for optimized code. You want to have a real exception here.

@lewisblake lewisblake merged commit 8086386 into main-dev Sep 30, 2024
6 of 8 checks passed
@lewisblake lewisblake deleted the extend-aeroval-satellite-pixels-maps branch September 30, 2024 11:28
@lewisblake lewisblake restored the extend-aeroval-satellite-pixels-maps branch October 1, 2024 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-feature SESAM Issues related to the SESAM project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AeroVal maps feature for raster data
3 participants