-
Notifications
You must be signed in to change notification settings - Fork 68
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
BUG #1886: Polar projection does not change pole. #1904
Conversation
Baselines in |
@doutriaux1 @aashish24 Please review. See also the issues observed while debugging this: |
The following tests still fail on master and on this branch: |
@danlipsa why there is a hole in test_vcs_basic_isofill_-3_proj_gmflip_NH_via_gm.png? |
# We don't use the zooming feature (gm.datawc) for geographic | ||
# projections. We use wrapped coordinates for doing the projection | ||
# We use zooming feature (gm.datawc) for linear and ploar projections. | ||
# We use wrapped coordinates for doing the projection |
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 sentence does not read well --> such that parameters such that central meridian are set correctly
@danlipsa could you please clean it up?
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.
Sure, I'll cleanup the comment. The command for test_vcs_basic_isofill_-3_proj_gmflip_NH_via_gm.png is
"/home/danlipsa/src/uvcdat/testing/vcs/test_vcs_basic_gms.py" "--gm_type=isofill" "--projection=-3" "--lat1=90" "--lat2=0" "--range_via_gm" "--gm_flips_lat_range" "--source=/home/danlipsa/build/uvcdat/uvcdat-testdata/baselines/vcs/test_vcs_basic_isofill_-3_proj_gmflip_NH_via_gm.png"
so
(90, 0) flipped -> (0, 90).
which means view from the South.
The hole is (-90, 0). Note the NH which is appended to the file is wrong: #1903
@danlipsa I have some questions but I see the pattern for meshfill which seems reasonable to me but I will have detailed look after I get the answers. |
@danlipsa meshfill will be fixed in a separate PR correct? |
@doutriaux1 Yes, we can definitely work on fixing the meshfill. #1905 |
Ok I'll test the updated PR and merge in then. |
@doutriaux1 Note meshfill might be trickier to get working as zooming does not work correctly for it: |
@doutriaux1 @aashish24 Do I merge this in or any of you want to merge it? |
Polar projection should use: South Pole for y1 < y2 North Pole for y1 > y2.
Polar projection should use:
South Pole for y1 < y2
North Pole for y1 > y2.