-
Notifications
You must be signed in to change notification settings - Fork 97
Fix derivation of correct radius of influence when data layout is not standard #555
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
#!/usr/bin/env python | ||
# -*- coding: utf-8 -*- | ||
# | ||
# Copyright (C) 2010-2020 Pyresample developers | ||
# Copyright (C) 2010-2023 Pyresample developers | ||
# | ||
# This program is free software: you can redistribute it and/or modify it under | ||
# the terms of the GNU Lesser General Public License as published by the Free | ||
|
@@ -28,6 +28,7 @@ | |
from typing import Optional, Sequence, Union | ||
|
||
import numpy as np | ||
import xarray as xr | ||
import pyproj | ||
import yaml | ||
from pyproj import Geod, Proj | ||
|
@@ -693,16 +694,29 @@ def geocentric_resolution(self, ellps='WGS84', radius=None, nadir_factor=2): | |
if self.ndim == 1: | ||
raise RuntimeError("Can't confidently determine geocentric " | ||
"resolution for 1D swath.") | ||
rows = self.shape[0] | ||
|
||
if isinstance(self.lons, xr.DataArray): | ||
rows = self.lons['y'].shape[0] | ||
else: | ||
# Data have no information on dimensions, so we assume first dimension (the rows) is the y-axis: | ||
logger.warning('As Numpy data arrays carry no information on the data layout we here ' + | ||
'assume the first dimension (the rows) is the y-axis (the satellite scans)') | ||
rows = self.shape[0] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So users get a warning for every execution of this method? No, we can't do this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lol right, sure, but the overall point still stands. I would still prefer that users receive no warning for a usage that has existed for years and years and is perfectly fine in 99.9999% of cases that we run into. My preference is still that the Satpy reader reorient data, but I also have no experience with that instrument. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok I take back the "years and years" thing, I thought this was a different method being modified...but still. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree that in this particular case, there should be no warning at all. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ha, very good, I did trigger a reaction here! I was pretty sure you wouldn't like this when I wrote it, but wanted to discuss it. Done now. So sorry, I take it back again. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ha, very good, I did trigger a reaction here! I was pretty sure you wouldn't like this when I wrote it, but wanted to discuss it. Done now. So sorry, I take it back again. |
||
|
||
start_row = rows // 2 # middle row | ||
src = CRS('+proj=latlong +datum=WGS84') | ||
if radius: | ||
dst = CRS("+proj=cart +a={} +b={}".format(radius, radius)) | ||
else: | ||
dst = CRS("+proj=cart +ellps={}".format(ellps)) | ||
# simply take the first two columns of the middle of the swath | ||
lons = self.lons[start_row: start_row + 1, :2] | ||
lats = self.lats[start_row: start_row + 1, :2] | ||
if isinstance(self.lons, xr.DataArray): | ||
lons = self.lons.sel(y=start_row)[:2] | ||
lats = self.lats.sel(y=start_row)[:2] | ||
else: | ||
lons = self.lons[start_row: start_row + 1, :2] | ||
lats = self.lats[start_row: start_row + 1, :2] | ||
|
||
if hasattr(lons.data, 'compute'): | ||
# dask arrays, compute them together | ||
import dask.array as da | ||
|
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.
This makes xarray a hard requirement on pyresample which it is not. Plus
DataArray
is already imported in a try/except below.