Skip to content
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

coherence change detection: bugfix + folium plot #49

Merged
merged 30 commits into from
Dec 23, 2022

Conversation

mgovorcin
Copy link
Contributor

  • fix few bugs in plotting function plot_ccd
  • replaced ax.invert with mintpy.utils.plot.auto_flip_direction

mgovorcin and others added 17 commits February 16, 2022 11:48
- applied Yunjun commits
- make the notebook shorter
change method option definition
from: 'diff', 'hist', 'ratio' to 'difference', histogram_matching', 'ratio'

make the code for plot function shorter and add option to flip axes if option orbit_direction used
Little cleanup following PEP8 style and renaming of variables to fit snake_case style
 - replaced "ax.invert_yaxis()" with mintpy.utils.plot.auto_flip_direction
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@review-notebook-app
Copy link

review-notebook-app bot commented Dec 22, 2022

View / edit / reply to this conversation on ReviewNB

yunjunz commented on 2022-12-22T01:19:49Z
----------------------------------------------------------------

I suggest to merge this cell with the cell below, for compactness.


@review-notebook-app
Copy link

review-notebook-app bot commented Dec 22, 2022

View / edit / reply to this conversation on ReviewNB

yunjunz commented on 2022-12-22T01:19:50Z
----------------------------------------------------------------

Line #174.        for i in range(0, nimg):

Use for i in range(nimg): instead.


mgovorcin commented on 2022-12-22T03:22:18Z
----------------------------------------------------------------

applied

@review-notebook-app
Copy link

review-notebook-app bot commented Dec 22, 2022

View / edit / reply to this conversation on ReviewNB

yunjunz commented on 2022-12-22T01:19:51Z
----------------------------------------------------------------

Line #6.    stack = os.path.join(work_dir, 'inputs/ifgramStack.h5')

I would suggest using stack_file to avoid potential ambiguity.


mgovorcin commented on 2022-12-22T03:22:31Z
----------------------------------------------------------------

done

@review-notebook-app
Copy link

review-notebook-app bot commented Dec 22, 2022

View / edit / reply to this conversation on ReviewNB

yunjunz commented on 2022-12-22T01:19:51Z
----------------------------------------------------------------

Line #35.    for i, method in enumerate(methods):

Use for method in methods: instead, since i is not used. And please check other occurrences.


mgovorcin commented on 2022-12-22T03:22:43Z
----------------------------------------------------------------

applied

@yunjunz
Copy link
Member

yunjunz commented Dec 22, 2022

Thank you @mgovorcin. Looks great overall. I only have a few minor suggestions on details.

Copy link
Contributor Author

applied


View entire conversation on ReviewNB

Copy link
Contributor Author

done


View entire conversation on ReviewNB

Copy link
Contributor Author

applied


View entire conversation on ReviewNB

@yunjunz
Copy link
Member

yunjunz commented Dec 22, 2022

Thanks @mgovorcin. One last thing: The google earth screen shot is very intuitive. I would highly recommend keeping it.

from IPython.display import Image
Image("Fernandina_eruptionSep2017.png")

@yunjunz yunjunz self-requested a review December 22, 2022 20:59
Copy link
Member

@yunjunz yunjunz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @mgovorcin. The folium plot is beautiful! Looks all good to me, squash and merge whenever you are ready please.

@yunjunz yunjunz changed the title Fixed bugs in plotting CCD CCD: bugfix + folium plot Dec 22, 2022
@yunjunz yunjunz changed the title CCD: bugfix + folium plot coherence change detection: bugfix + folium plot Dec 22, 2022
@mgovorcin mgovorcin merged commit abbaf55 into insarlab:main Dec 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants