-
Notifications
You must be signed in to change notification settings - Fork 27
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
Vector support in bridge.Transform.from_points #676
base: main
Are you sure you want to change the base?
Changes from 12 commits
bd8c868
7fc4309
9b6ca8e
f6823ac
a05ff4b
82155a0
0f1ab54
58dc72a
5167fdb
6037a1e
48a57f3
1f1e994
baaaee1
2ad49bb
6e836e7
cf85be7
225cfd2
bc1ba77
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 |
---|---|---|
|
@@ -750,6 +750,51 @@ def to_cartesian( | |
return np.vstack(xyz).T if stacked else np.array(xyz) | ||
|
||
|
||
def vectors_to_cartesian( | ||
lons_lats: (ArrayLike, ArrayLike), | ||
vectors_uvw: (ArrayLike, ArrayLike, ArrayLike), | ||
) -> (np.ndarray, np.ndarray, np.ndarray): | ||
"""Convert geographic-oriented vectors to cartesian ``xyz`` points. | ||
|
||
Parameters | ||
---------- | ||
lons_lats : pair of ArrayLike | ||
The longitude + latitude locations of the vectors (in degrees). | ||
Both shapes must be the same. | ||
vectors_uvw : triple of ArrayLike | ||
The eastward, northward and upward vector components. | ||
All shapes must be the same as in ``lons_lats``. | ||
|
||
Returns | ||
------- | ||
(ndarray, ndarray, ndarray) | ||
The corresponding ``xyz`` cartesian vector components. | ||
|
||
Notes | ||
----- | ||
.. versionadded:: 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. I assume this gets gets fixed once the PR is completed/approved. |
||
|
||
""" | ||
# TODO @pp-mo: Argument checking ??? | ||
lons, lats = (np.deg2rad(arr) for arr in lons_lats) | ||
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. It may be outside the scope of this PR, but it occurs to me that the documentation of these functions could be a bit clearer on the fact that, unless a crs is specified, units are expected to be in degrees. Though I suppose this may be implicit in the default crs being 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 felt that all the existing docs of Transform methods are missing any statement of this, so it just didn't seem an appropriate place to mention it. 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. Actually, not true I see : in the dosctring here for from_points, it does say that |
||
u, v, w = vectors_uvw | ||
|
||
coslons = np.cos(lons) | ||
sinlons = np.sin(lons) | ||
coslats = np.cos(lats) | ||
sinlats = np.sin(lats) | ||
# N.B. the term signs are slightly unexpected here, because the viewing coord system | ||
# is not quite what you may expect : The "Y" axis goes to the right, and the "X" | ||
# axis points out of the screen, towards the viewer. | ||
z_factor = w * coslats - v * sinlats | ||
wy = coslons * u + sinlons * z_factor | ||
wx = -sinlons * u + coslons * z_factor | ||
wz = v * coslats + w * sinlats | ||
# NOTE: for better efficiency, we *COULD* handle the w=0 special case separately. | ||
|
||
return wx, wy, wz | ||
|
||
|
||
def to_lonlat( | ||
xyz: ArrayLike, | ||
radians: bool | None = False, | ||
|
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.
Given that there is a case for
vectors_to_cartesian
to be used in other transforms likefrom_unstructured
, would it be worth considering if this code (from line 659 to here) would be suitable to exist insidevectors_to_cartesian
? It doesn't seem to me like this code is specific tofrom_1d
and I don't believe there's anything here which wouldn't work with the other transforms from what I've seen of them.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.
Interesting...
I'm actually doubting right now whether we may not remove this keyword altogether, since it's so obvious how the user would do it.
I already removed an overall scaling operation for the same reason, and also because it can be done in the 'glyph' call.