switch coherence matrix plot from index-axis to time-axis#1495
Merged
yunjunz merged 7 commits intoJul 20, 2026
Merged
Conversation
Contributor
Reviewer's GuideAdds a new continuous time-axis coherence matrix plot function and integrates it into both the interactive coherence-matrix viewer and the plot_network workflow, including a new saved figure and a CLI flag to toggle the new view. Flow diagram for coherence matrix plotting with continuous time-axis optionflowchart LR
subgraph Interactive_viewer
CLI_plot_coherence_matrix["cli/plot_coherence_matrix.py
parse args (including --time-axis)"]
PlotCoherenceMatrixClass["plot_coherence_matrix.CoherenceMatrixPlotter"]
Method_plot_coh["plot_coherence_matrix.plot_coherence_matrix4pixel"]
Method_plot_coh_time["plot_coherence_matrix.plot_coherence_matrix4pixel_time_axis"]
end
subgraph Core_plotting
Util_plot_time_axis["mintpy.utils.plot.plot_coherence_matrix_time_axis"]
end
subgraph Network_workflow
Plot_network["plot_network.plot_network"]
Fig4_time_axis["Fig 4: coherenceMatrixTimeAxis.pdf"]
Fig5_network["Fig 5: network (or Fig 4 if cohList is None)"]
end
CLI_plot_coherence_matrix --> PlotCoherenceMatrixClass
PlotCoherenceMatrixClass -->|time_axis == False| Method_plot_coh
PlotCoherenceMatrixClass -->|time_axis == True| Method_plot_coh_time
Method_plot_coh --> Util_plot_time_axis
Method_plot_coh_time --> Util_plot_time_axis
Plot_network -->|cohList is not None| Util_plot_time_axis
Util_plot_time_axis -->|pcolormesh on date grid| Fig4_time_axis
Plot_network --> Fig5_network
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- The legend entries and annotations for the time-axis coherence matrix still refer to "upper = used / lower = all" interferograms, but this implementation only uses a single matrix with dropped pairs set to NaN, so the legend text is misleading and should be updated or removed where enabled (e.g., in plot_network).
- plot_coherence_matrix_time_axis duplicates quite a bit of setup/formatting logic from plot_coherence_matrix; consider factoring out common pieces (e.g., default p_dict handling, colormap and colorbar setup) into a shared helper to reduce divergence and make future changes easier.
- In plot_network, you pass vars(inps) directly as p_dict into plot_coherence_matrix_time_axis; if inps gains additional attributes with names overlapping the expected keys (e.g., colormap, vlim, legend_loc), they will silently override the intended defaults, so it may be safer to build a smaller dict with only the keys that plot_coherence_matrix_time_axis uses.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The legend entries and annotations for the time-axis coherence matrix still refer to "upper = used / lower = all" interferograms, but this implementation only uses a single matrix with dropped pairs set to NaN, so the legend text is misleading and should be updated or removed where enabled (e.g., in plot_network).
- plot_coherence_matrix_time_axis duplicates quite a bit of setup/formatting logic from plot_coherence_matrix; consider factoring out common pieces (e.g., default p_dict handling, colormap and colorbar setup) into a shared helper to reduce divergence and make future changes easier.
- In plot_network, you pass vars(inps) directly as p_dict into plot_coherence_matrix_time_axis; if inps gains additional attributes with names overlapping the expected keys (e.g., colormap, vlim, legend_loc), they will silently override the intended defaults, so it may be safer to build a smaller dict with only the keys that plot_coherence_matrix_time_axis uses.
## Individual Comments
### Comment 1
<location path="src/mintpy/utils/plot.py" line_range="962" />
<code_context>
return ax, coh_mat, im
+def plot_coherence_matrix_time_axis(ax, date12List, cohList, date12List_drop=[], p_dict={}):
+ """Plot Coherence Matrix with continuous time axis
+ Parameters: ax : matplotlib.pyplot.Axes,
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid using a mutable dict as the default value for p_dict.
`{}` as a default argument makes all calls share the same dict instance, which can cause subtle bugs if `p_dict` is mutated. Prefer `p_dict=None` and then inside: `p_dict = {} if p_dict is None else dict(p_dict)`.
</issue_to_address>
### Comment 2
<location path="src/mintpy/utils/plot.py" line_range="1001-1005" />
<code_context>
+ date12List_drop = ptime.yyyymmdd_date12(date12List_drop)
+ for date12 in date12List_drop:
+ idx1, idx2 = (dateList.index(i) for i in date12.split('_'))
+ coh_mat[idx1, idx2] = np.nan
+
+ if len(dateList_dt) == 1:
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Dropped interferograms are only masked on one triangle of the symmetric matrix.
Because `coherence_matrix` is symmetric, updating only `coh_mat[idx1, idx2]` leaves `coh_mat[idx2, idx1]` unmasked, which can produce inconsistent plots. Please mask both `[idx1, idx2]` and `[idx2, idx1]` to keep the visualization symmetric.
```suggestion
if date12List_drop:
date12List_drop = ptime.yyyymmdd_date12(date12List_drop)
for date12 in date12List_drop:
idx1, idx2 = (dateList.index(i) for i in date12.split('_'))
coh_mat[idx1, idx2] = np.nan
coh_mat[idx2, idx1] = np.nan
```
</issue_to_address>
### Comment 3
<location path="src/mintpy/utils/plot.py" line_range="1092" />
<code_context>
+ ax.set_xlabel('Time', fontsize=fs, labelpad=18)
+ ax.set_ylabel('Time', fontsize=fs, labelpad=18)
+ ax.set_aspect('equal', adjustable='box')
+ ax.invert_yaxis()
+
+ if p_dict['disp_title']:
</code_context>
<issue_to_address>
**issue (bug_risk):** Inverting the y-axis after placing month/year labels likely moves them away from the intended edges.
Because the month/year positions use `ax.get_ylim()[1]` and `ax.get_xlim()[0]` computed before `invert_yaxis()`, those values no longer map to the top/left edges after inversion, so the labels can shift toward the bottom or misalign. Either invert the y-axis before computing `offset`/placing the labels, or place them in axis coordinates (`transform=ax.transAxes`) so they stay fixed to the edges regardless of axis direction.
</issue_to_address>
### Comment 4
<location path="src/mintpy/plot_coherence_matrix.py" line_range="221" />
<code_context>
+ self.fig_img.canvas.draw_idle()
+ return
+
def plot_coherence_matrix4pixel(self, yx):
"""Plot coherence matrix for one pixel
Parameters: yx : list of 2 int
</code_context>
<issue_to_address>
**issue (complexity):** Consider unifying the two pixel plotting paths into a single parameterized `plot_coherence_matrix4pixel` function that selects the appropriate plotting routine based on `time_axis` while sharing all common setup and marker logic.
You can remove most of the duplication by parameterizing the plot function and keeping a single `plot_coherence_matrix4pixel` implementation. That also lets you unify the layout and marker logic.
For example:
```python
def plot_coherence_matrix4pixel(self, yx):
"""Plot coherence matrix for one pixel."""
# choose plotting function based on time_axis
plot_func = (
pp.plot_coherence_matrix_time_axis
if self.time_axis
else pp.plot_coherence_matrix
)
self.ax_mat.cla()
# read coherence
box = (yx[1], yx[0], yx[1] + 1, yx[0] + 1)
coh = readfile.read(self.ifgram_file, datasetName='coherence', box=box)[0]
# ex_date for pixel-wise masking during network inversion
ex_date12_list = self.ex_date12_list[:] # local copy
if self.min_coh_used > 0.0:
ex_date12_list += np.array(self.date12_list)[coh < self.min_coh_used].tolist()
ex_date12_list = sorted(set(ex_date12_list))
# common plot config
plotDict = {
"fig_title": f"Y = {yx[0]}, X = {yx[1]}",
"colormap": self.colormap,
"cbar_label": "Coherence",
"disp_legend": False,
}
if self.tcoh is not None:
tcoh = self.tcoh[yx[0], yx[1]]
plotDict["fig_title"] += f", tcoh = {tcoh:.2f}"
if len(self.cmap_vlist) >= 2:
plotDict["vlim"] = [self.cmap_vlist[0], self.cmap_vlist[-1]]
else:
plotDict["vlim"] = [0.0, 1.0]
# call the selected plotting function
plot_func(
self.ax_mat,
date12List=self.date12_list,
cohList=coh.tolist(),
date12List_drop=ex_date12_list,
p_dict=plotDict,
)
# common annotations
self.ax_mat.annotate(
"ifgrams\navailable",
xy=(0.05, 0.05),
xycoords="axes fraction",
fontsize=12,
)
self.ax_mat.annotate(
"ifgrams\nused",
ha="right",
xy=(0.95, 0.85),
xycoords="axes fraction",
fontsize=12,
)
msg = f"pixel in yx = {tuple(yx)}, "
msg += (
f"min/max spatial coherence: {np.nanmin(coh):.2f} / {np.nanmax(coh):.2f}, "
)
if self.tcoh is not None:
msg += f"temporal coherence: {tcoh:.2f}"
vprint(msg)
self.fig_mat.canvas.manager.set_window_title(self.figname_mat)
if not hasattr(self, "_mat_tight_layout_done"):
self.fig_mat.tight_layout()
self._mat_tight_layout_done = True
self.fig_mat.canvas.draw_idle()
self.fig_mat.canvas.flush_events()
# common marker logic
if self.fig_coord == "geo":
lat, lon = self.coord.yx2lalo(yx[0], yx[1])
mx, my = lon, lat
else:
mx, my = yx[1], yx[0]
if self._marker_artist is None:
self._marker_artist = self.ax_img.plot(
mx, my, "r^", markersize=6, markeredgecolor="black"
)[0]
else:
self._marker_artist.set_data([mx], [my])
self.fig_img.canvas.draw_idle()
```
Actionable steps:
1. Remove `plot_coherence_matrix4pixel_time_axis` entirely.
2. Fold its unique behavior into `plot_coherence_matrix4pixel` by:
- Selecting `plot_func` based on `self.time_axis`.
- Reusing shared logic for:
- reading `coh` / building `ex_date12_list`
- constructing `plotDict` (including `tcoh`)
- annotations, logging, and window title
- tight layout (`_mat_tight_layout_done`) for both paths
- marker update on `ax_img`.
3. Keep the existing public API (`plot_coherence_matrix4pixel` signature) and the `time_axis` flag, so no external behavior changes.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| self.fig_img.canvas.draw_idle() | ||
| return | ||
|
|
||
| def plot_coherence_matrix4pixel(self, yx): |
Contributor
There was a problem hiding this comment.
issue (complexity): Consider unifying the two pixel plotting paths into a single parameterized plot_coherence_matrix4pixel function that selects the appropriate plotting routine based on time_axis while sharing all common setup and marker logic.
You can remove most of the duplication by parameterizing the plot function and keeping a single plot_coherence_matrix4pixel implementation. That also lets you unify the layout and marker logic.
For example:
def plot_coherence_matrix4pixel(self, yx):
"""Plot coherence matrix for one pixel."""
# choose plotting function based on time_axis
plot_func = (
pp.plot_coherence_matrix_time_axis
if self.time_axis
else pp.plot_coherence_matrix
)
self.ax_mat.cla()
# read coherence
box = (yx[1], yx[0], yx[1] + 1, yx[0] + 1)
coh = readfile.read(self.ifgram_file, datasetName='coherence', box=box)[0]
# ex_date for pixel-wise masking during network inversion
ex_date12_list = self.ex_date12_list[:] # local copy
if self.min_coh_used > 0.0:
ex_date12_list += np.array(self.date12_list)[coh < self.min_coh_used].tolist()
ex_date12_list = sorted(set(ex_date12_list))
# common plot config
plotDict = {
"fig_title": f"Y = {yx[0]}, X = {yx[1]}",
"colormap": self.colormap,
"cbar_label": "Coherence",
"disp_legend": False,
}
if self.tcoh is not None:
tcoh = self.tcoh[yx[0], yx[1]]
plotDict["fig_title"] += f", tcoh = {tcoh:.2f}"
if len(self.cmap_vlist) >= 2:
plotDict["vlim"] = [self.cmap_vlist[0], self.cmap_vlist[-1]]
else:
plotDict["vlim"] = [0.0, 1.0]
# call the selected plotting function
plot_func(
self.ax_mat,
date12List=self.date12_list,
cohList=coh.tolist(),
date12List_drop=ex_date12_list,
p_dict=plotDict,
)
# common annotations
self.ax_mat.annotate(
"ifgrams\navailable",
xy=(0.05, 0.05),
xycoords="axes fraction",
fontsize=12,
)
self.ax_mat.annotate(
"ifgrams\nused",
ha="right",
xy=(0.95, 0.85),
xycoords="axes fraction",
fontsize=12,
)
msg = f"pixel in yx = {tuple(yx)}, "
msg += (
f"min/max spatial coherence: {np.nanmin(coh):.2f} / {np.nanmax(coh):.2f}, "
)
if self.tcoh is not None:
msg += f"temporal coherence: {tcoh:.2f}"
vprint(msg)
self.fig_mat.canvas.manager.set_window_title(self.figname_mat)
if not hasattr(self, "_mat_tight_layout_done"):
self.fig_mat.tight_layout()
self._mat_tight_layout_done = True
self.fig_mat.canvas.draw_idle()
self.fig_mat.canvas.flush_events()
# common marker logic
if self.fig_coord == "geo":
lat, lon = self.coord.yx2lalo(yx[0], yx[1])
mx, my = lon, lat
else:
mx, my = yx[1], yx[0]
if self._marker_artist is None:
self._marker_artist = self.ax_img.plot(
mx, my, "r^", markersize=6, markeredgecolor="black"
)[0]
else:
self._marker_artist.set_data([mx], [my])
self.fig_img.canvas.draw_idle()Actionable steps:
- Remove
plot_coherence_matrix4pixel_time_axisentirely. - Fold its unique behavior into
plot_coherence_matrix4pixelby:- Selecting
plot_funcbased onself.time_axis. - Reusing shared logic for:
- reading
coh/ buildingex_date12_list - constructing
plotDict(includingtcoh) - annotations, logging, and window title
- tight layout (
_mat_tight_layout_done) for both paths - marker update on
ax_img.
- reading
- Selecting
- Keep the existing public API (
plot_coherence_matrix4pixelsignature) and thetime_axisflag, so no external behavior changes.
huchangyang
force-pushed
the
feature/coherence-matrix-time-axis-clean
branch
9 times, most recently
from
July 10, 2026 06:22
3130a5b to
52c65fe
Compare
Add a date-centered coherence matrix with year tick labels reused from
auto_adjust_xaxis_date(), expose --axis-format {index,time}, and include
the time-axis matrix in plot_network output.
For spans within 1.5 years, place year at January and odd-month numbers on the same tick line; longer spans keep year-only labels.
huchangyang
force-pushed
the
feature/coherence-matrix-time-axis-clean
branch
from
July 13, 2026 06:39
52c65fe to
e98f518
Compare
Expose mintpy.network.plotAxisFormat (auto->time), keep plot_network at four figures with a selectable matrix axis, and unify format_coord in plot_coherence_matrix helpers.
format_coord now lives in the plot helpers, so the returned matrix is unused.
yunjunz
changed the title
yunjunz
approved these changes
Jul 20, 2026
Member
There was a problem hiding this comment.
Thank you @huchangyang, looks all good to me. I am excited to see this new change!
- Old style via
plot_network.py --ax-fmt index:
- New style via
plot_network.py --ax-fmt time, which is the new default:
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of proposed changes
This PR adds a continuous time-axis view for the per-pixel coherence matrix, complementing the existing date-index matrix from #1493.
Motivation: The index-based matrix uses acquisition order (0, 1, 2, …), which is hard to read when dates are irregular. A time-axis view maps each acquisition to its calendar date and uses date-centered cell edges, so temporal gaps are visible in the matrix layout.
Changes:
mintpy.utils.plot.plot_coherence_matrix_time_axis()New plotting function (same pattern as
plot_coherence_matrix()):pcolormeshon a date-centered grid, year tick labels reused fromauto_adjust_xaxis_date(), diagonal masking, dropped-ifgram handling, colorbar, and status-barformat_coord.plot_network.pyAdds Figure 4 —
coherenceMatrixTimeAxis.pdf— when coherence lists are available.plot_coherence_matrix.py(interactive)Adds
--axis-format {index,time}(default:index) to switch the matrix window between date-index and continuous time-axis views; map window and click-to-update behavior are unchanged.cli/plot_coherence_matrix.pyRegisters
--axis-formatonlyMatrix plotting method
The time-axis matrix is drawn as follows:
Date-centered grid
Each acquisition is placed at the midpoint between neighboring cell edges. For dates
d0, d1, …, dn:(di + di+1) / 2d0 - (d1-d0)/2anddn + (dn-dn-1)/2,so edge cells have the same width as interior cells.
pcolormeshon matplotlib date numbersGrid edges are converted with
mdates.date2num()becausepcolormesh()needs numeric vertex coordinates on datetime axes. The coherence matrix is plotted on this grid; the diagonal is drawn in gray to separate selected vs. unselected interferograms; dropped pairs are set to NaN (shown as white).Axis formatting
Year major ticks / month minor ticks are taken from
auto_adjust_xaxis_date()and applied to both X and Y. Axis titles areTime(to match the index matrix’sImage Number). Y-axis year labels are rotated 90°.Examples
Interactive viewer:
plot_coherence_matrix.py inputs/ifgramStack.h5 --axis-format timeWhen time series span longer than 1.5 years, the month label will not be shwon.
Reminders
Summary by Sourcery
Add a continuous time-axis coherence matrix view and integrate it into both the interactive coherence viewer and network plotting workflow.
New Features:
Enhancements: