Skip to content

Adding create_movie() in ROMSOutput class + support in plot.py#611

Open
blsaenz wants to merge 4 commits into
mainfrom
ROMSOutput-movies
Open

Adding create_movie() in ROMSOutput class + support in plot.py#611
blsaenz wants to merge 4 commits into
mainfrom
ROMSOutput-movies

Conversation

@blsaenz

@blsaenz blsaenz commented May 30, 2026

Copy link
Copy Markdown
Contributor

This PR adds movie-making abilities to ROMSoutput (ROMSoutput.create_movie()), with similar inputs as the plot() method. It builds upon the plot() functionality in plot.py, but required a bit of abstracting and refactoring to re-use that code.

Example:

cdr_output.create_movie(
"ALK_source",
time_range=slice(0, -1, 6), # every 6 output (here, every six hours)
fps=6, # frames per second
output_file="simulation_ALK_source.mp4",
s=-1, # surface
include_boundary=True, # include boundary grid cells
timestamp_xy=(0.05, 0.95) # display a time stamp at plot axes (x,y)
)

  • Tests added
  • Passes pre-commit run --all-files
  • Changes are documented in docs/releases.md
  • New functions/methods are listed in docs/api.rst
  • New functionality has documentation

@blsaenz

blsaenz commented May 30, 2026

Copy link
Copy Markdown
Contributor Author

Adding PR for visibility. I still need to create documentation.

@blsaenz blsaenz self-assigned this May 30, 2026
@blsaenz blsaenz marked this pull request as draft May 30, 2026 15:27
@blsaenz blsaenz requested review from kthyng and smaticka May 30, 2026 15:28
@codecov

codecov Bot commented May 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 37.58389% with 93 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.82%. Comparing base (e76a238) to head (1874441).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
roms_tools/analysis/roms_output.py 16.92% 54 Missing ⚠️
roms_tools/plot.py 58.66% 28 Missing and 3 partials ⚠️
roms_tools/regrid.py 11.11% 8 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (e76a238) and HEAD (1874441). Click for more details.

HEAD has 4 uploads less than BASE
Flag BASE (e76a238) HEAD (1874441)
without-dask 3 0
with-dask-1 1 0
@@             Coverage Diff             @@
##             main     #611       +/-   ##
===========================================
- Coverage   86.27%   54.82%   -31.46%     
===========================================
  Files          28       28               
  Lines        6171     6452      +281     
  Branches     1101     1151       +50     
===========================================
- Hits         5324     3537     -1787     
- Misses        564     2624     +2060     
- Partials      283      291        +8     
Flag Coverage Δ
glorys-regional-invariance 35.30% <21.47%> (-0.80%) ⬇️
with-dask-1 ?
with-dask-2 51.00% <37.58%> (-0.69%) ⬇️
with-dask-and-xesmf-2 54.33% <37.58%> (-0.84%) ⬇️
with-streaming 35.08% <21.47%> (-0.79%) ⬇️
without-dask ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
roms_tools/regrid.py 79.59% <11.11%> (-20.41%) ⬇️
roms_tools/plot.py 46.73% <58.66%> (-32.15%) ⬇️
roms_tools/analysis/roms_output.py 14.12% <16.92%> (-18.92%) ⬇️

... and 24 files with indirect coverage changes

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

@kthyng kthyng left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it a good time to try this or are there more changes coming to wait for? I haven't tried it yet but would like to!

)

@staticmethod
def _cmap_name_for_var(var_name: str) -> str:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I have a package called xcmocean that I haven't looked at for awhile (https://github.com/pangeo-data/xcmocean) which is meant to do this job in a more general way: use regex to match variable names to the likely intended cmocean colormap. It's a bit kludgey with the variable matching, but has some good parts.

@blsaenz blsaenz Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We can consider it but maybe in a differenct PR? This function is actually just a re-wrapping of what ROMS-Tools does now for plot() for picking a colormap, so I am not introducing anything new or exciting here. I don't even know if we can use a custom colormap. Design-wise, I am under the impression we are not trying to support 'fancy' plotting options, but it would b good to bring up in standup or backlog grooming since I don't know for sure.

field.load()
return field

def create_movie(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

does this also save the individual images that are being used to create the animation? I often like to use those individually too (then link with ffmpeg), personally, so it might be nice if that is possible with this approach.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, in fact I used the Animation class in matplotlib specifically to avoid creating gigabytes of individual plots. I think it's cleanest just to give what is is the name - a single movie . And then if you find a frame where an individual plot is needed, it is easy to call plot() with the same parameters to generate it the time/slice where you need it?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah fair enough!

@review-notebook-app

Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@blsaenz

blsaenz commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

Updated documentation! Ready for full review I think

@blsaenz blsaenz marked this pull request as ready for review June 8, 2026 16:08

@kthyng kthyng left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks reasonable.

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