plot: --mask-gps/dem, --gps-comp horz/vert and American display unit#670
plot: --mask-gps/dem, --gps-comp horz/vert and American display unit#670yunjunz merged 31 commits intoinsarlab:mainfrom
Conversation
|
Regarding |
|
Thank you for the PR @sssangha, it's much more clear. Feature 4 looks good to me. Could you address the suggestions I made on features 1 and 2? Let me know if there is any place unclear, I would be more than happy to elaborate. After that, I will check feature 3, as it has related code to feature 2. |
|
@yunjunz sounds good, I'm preparing a push soon where I integrate the suggestions. I will confirm with you once the checks pass for you to review. Thanks again |
|
@yunjunz ok, looks like I successfully sorted out some codacy snags! Let me know what you think of my updates. |
+ get_gps_los_obs(): remove ref_site to support spatially absolute comparison. + view.py: - turn OFF auto mask file detection when --zero-mask is specified. + viewer.plot(): save self.msk for single plot, grabbing both nan value and mask matrix if available
yunjunz
changed the title
|
This PR looks all good to me now. Thank you @sssangha for the many new features! You and I spent quite some time during this and previous PR to figure the flow of changes. They really should be broken down into separate PRs (one PR for one new feature or behavior change) next time. You might want to do the same as much as possible for your own commits as well. |
Yes, my apologies. There were a series of old changes I built off a ~1 yr old fork and I was really -- probably a bit too much so -- motivated to get them all in one shot. With that in mind, I actually am preparing to push a simple PR to correct a bug I found related to one specific aspect of this PR! Will again update you when it's ready. |
|
No worries at all, and thank you again for the features. They do look good in the plotting! |
2 participants
Description of proposed changes
Following discussion in PR #666 , I plan to push two separate and successive PRs, starting with this one which introduces the following changes/options:
1. Option to Mask DEM background, where it falls outside of InSAR coverage or is coincident with masked/nodata values (
--mask-dem).2. Option to mask GPS stations that fall outside of InSAR coverage or are coincident with masked/nodata values (
--mask-gps).3. Introduce support for plotting vertical or horizontal GPS components if the user is plotting vertical or horizontal InSAR velocity components, respectively, via
--gps-comp horz/vertand--horz-azoptions.4. Update list of supported units to also support American convention (e.g. inches, feet, miles).
@yunjunz instead of issuing the other PR I want right now -- which supports plotting of expanding bounds -- I think it would be easier to first have this PR merged. Plus perhaps for reference we should keep PR #666 open for now, until both proposed PRs are merged? What do you think?
Reminders