Skip to content

Masking in Topo Editor + Rectangle Selector#83

Open
manishvenu wants to merge 6 commits into
mainfrom
mask_te_2
Open

Masking in Topo Editor + Rectangle Selector#83
manishvenu wants to merge 6 commits into
mainfrom
mask_te_2

Conversation

@manishvenu
Copy link
Copy Markdown
Collaborator

@manishvenu manishvenu commented Apr 30, 2026

Replaces #76 , Closes CROCODILE-CESM/CrocoDash#134

Changes:

Add the mask variable to the topo editor in the form of using the topo mask property and adding a mask edit field that can be switched from land to ocean
Add a toggle to do rectangle select, to set large chunks of bathymetry to a specific value
Minor cleaning

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 30, 2026

📄 Preview your docs here:
👉 https://NCAR.github.io/mom6_forge/pr-83/index.html

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 30, 2026

Codecov Report

❌ Patch coverage is 80.37383% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 46.24%. Comparing base (46c0103) to head (b43090b).
⚠️ Report is 12 commits behind head on main.

Files with missing lines Patch % Lines
mom6_forge/topo_editor.py 80.37% 13 Missing and 8 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #83      +/-   ##
==========================================
+ Coverage   40.71%   46.24%   +5.52%     
==========================================
  Files          14       14              
  Lines        2655     2742      +87     
  Branches      274      287      +13     
==========================================
+ Hits         1081     1268     +187     
+ Misses       1529     1409     -120     
- Partials       45       65      +20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Member

@alperaltuntas alperaltuntas left a comment

Choose a reason for hiding this comment

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

I left a couple of comments, but more importantly, I am unable to test these changes because TopoEditor doesn't work in jupyter via the mom6_forge conda environment. I am getting:

ImportError: libpmi.so.0: cannot open shared object file: No such file or directory

when I attempt to run TopoEditor via the (newly installed) mom6_forge conda environment. We need to fix that issue first before I can evaluate this PR.

Comment thread mom6_forge/topo_editor.py
def active_cells(self):
if hasattr(self, "_selected_cells") and self._selected_cells:
return list(self._selected_cells)
elif self._selected_cell is not None:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This elif statement doesn't make sense to me. If the above if statement evaluates to true, then this elif branch will be skipped. If the if statement is false, then this elif branch will be false too, or will lead to a runtime error.

Copy link
Copy Markdown
Collaborator Author

@manishvenu manishvenu May 4, 2026

Choose a reason for hiding this comment

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

Ahh that's maybe my bad. The different is an "s": selected_cells, versus selected_cell. Want me to change that to more different?

Comment thread mom6_forge/topo_editor.py
# Git/domain controls
self._git_create_branch_button.on_click(self.on_git_create_branch)
self._git_checkout_button.on_click(self.on_git_checkout)
self._display_mode_toggle.observe(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is this observe call no longer necessary?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It was repeated twice.

@manishvenu
Copy link
Copy Markdown
Collaborator Author

@alperaltuntas, I am unable to reproduce your error directly, I tightened up the ipympl to be <0.10.0 and fixed a seperate error so that potentially may help.

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.

Change large areas in topography editor

3 participants