Thank you @sssangha for contributing (your data is beautiful!). All your 5 new features sound good to me in general. I have a few specific suggestions below that I hope you could address before considering merging the PR. Please do not let my strict review discourage you, view.py is one of the most used scripts, I just want to make sure it does not break.

1. both circle ci and codacy flagged a few issues, could you address them?
2. For DEM masking, could we add an option for it, e.g. --mask-dem with no masking as default?
3. For GPS masking, same as DEM.

These are a lot of changes in different scripts. With the current form, it's hard to track them and understand the reason behind the changes. Is it possible for you to break them down into independent PRs for each of the new features and add more comments?

Thank you @sssangha for contributing (your data is beautiful!). All your 5 new features sound good to me in general. I have a few specific suggestions below that I hope you could address before considering merging the PR. Please do not let my strict review discourage you, view.py is one of the most used scripts, I just want to make sure it does not break.

Thanks @yunjunz! And yes, I agree. We need to be careful with commits especially to view.py. Honestly, I'm always a bit nervous when committing to this script in particular in mintpy, and 'extractProduct.py' in ARIA-tools!

Anyways, I've integrated all of your suggestions and the tests seem to run fine locally, but it looks like I may need to sort out some circleci issues. I will confirm here once that all seems to be addressed.

@yunjunz it looks like I sorted out the codacy and circleCI issues. Please let me know if you have any other questions/concerns. Thanks again

@yunjunz I believe I have addressed all of your initial comments/concerns with my latest commits, so please let me know if any further action or review is required before we review. Thanks.

Thank you @sssangha. I will check your PR again tonight.

Hi @sssangha, I find it hard to see the flow of changes (and potential bugs), because the 1st feature requires complicated modifications (due to the complex script itself), while the rest 4 features are straightforward. Would you please consider separating this PR into two PRs: one for the extended bounds and one for the rest?

@yunjunz sure, I will take care of this later Tuesday.

Would it be possible to let me know though or help resolve potential conflicts if you plan on committing anything involved with the scripts in this PR, in case we both attempt commits today? Thanks

🙌🏼 No planned commit from me.

@sssangha I forgot to mention that subset.py supports extended bounds. You might want to check the code there also. It might be a good idea to have related new functions sit there. The existing functions are not well commented though, please feel free to modify them as needed.

@sssangha I forgot to mention that subset.py supports extended bounds. You might want to check the code there also. It might be a good idea to have related new functions sit there. The existing functions are not well commented though, please feel free to modify them as needed.

MintPy/mintpy/subset.py

Line 377 in 670c6a9

# Calculate Subset/Overlap Index

I was thinking about this, but the problem is that the relevant function generates a physical output file, which would lead to a great deal of overhead and inflexibility (e.g. imagine physical outputs for an entire TS stack and large DEM just for plotting purposes).

The proposed function I've written is pretty short and would adjust data bounds on the fly for plotting, but I could add the function to this script for organization purposes. What do you think?

I agree with you, the current form of sub-functions in subset.py is not good enough for plotting purposes.

The proposed function I've written is pretty short and would adjust data bounds on the fly for plotting, but I could add the function to this script for organization purposes. What do you think?

I like this idea.

Hi @sssangha, sorry for the late following up. With the other functionalities merged, shall we rebase this PR (or issue a new PR if you prefer) for the expanded bounds in view.py?

It will be great to have an example usage with a short explanation in the view.py#L45 as well.