Repository navigation
fix: use the executions list instead of latest_execution - #3
Conversation
The REF API removed latest_execution from execution groups. Notebooks 01 and 02 now pick the newest successful execution with a new latest_successful_execution helper, and print a message when there is none. Notebook 05 only fetches series for models with an ECS value and fetches them concurrently, because the API now has enough groups to exceed the cell timeout. Fixes #2
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 42 minutes. View limit details
Walkthrough
Priority: Change: Bug fix · Severity of issue fixed: Medium Merge Risk:
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Linked Issues check |  |
Issue #2 requires notebooks 01 and 02 to select the newest successful, unretracted execution from executions, show its output files, and show a helpful message without stopping when no successful ex… |
Guard or skip the dependent NetCDF and plot cells in notebook 02 when no successful execution exists. The notebook must complete with the helpful message instead of raising an error. |
| Out of Scope Changes check | |
The notebook 05 changes restrict tas requests by ECS availability and add batching. They address an independent runtime timeout and do not implement issue #2, which concerns execution selection and … |
Remove the notebook 05 performance changes and unrelated notebook output refreshes from this PR, or track them under a separate active coding objective and review them separately. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title accurately describes the main change: replacing the removed latest_execution field with the executions list and a helper for selecting a successful execution. Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 3 files. (1 skipped: 1 unsupported.)
-
Full details: Linked Issues check
-
Explanation
Issue
#2requires notebooks 01 and 02 to select the newest successful, unretracted execution fromexecutions, show its output files, and show a helpful message without stopping when no successful execution exists.latest_successful_executionfilterssuccessfulandretractedexecutions and selects the greatestcreated_at. Notebook 01 handlesNone. Notebook 02 selects a group with a successful execution and prints a message when none exists, but its later NetCDF cell still fails on that path. The helper has unit tests for newest eligible, no-success, and empty groups. The PR description also reports that all five notebooks passnbmakelocally.Resolution
Guard or skip the dependent NetCDF and plot cells in notebook 02 when no successful execution exists. The notebook must complete with the helpful message instead of raising an error.
Full details: Out of Scope Changes check
-
Explanation
The notebook 05 changes restrict
tasrequests by ECS availability and add batching. They address an independent runtime timeout and do not implement issue#2, which concerns execution selection and output handling in notebooks 01 and 02. The output-only refreshes for notebooks 03 and 04 are also not needed for the linked issue, although their connection to live API re-execution is stated in the PR description.Resolution
Remove the notebook 05 performance changes and unrelated notebook output refreshes from this PR, or track them under a separate active coding objective and review them separately.
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
…ebook 02 This keeps the download and plotting cells working when the first group has no successful run.
Outputs, execution counts and cell ids are kept, because the notebooks render on GitHub.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
02-querying-the-api.ipynb:945-960
notebooks/02-querying-the-api.ipynb:945-960
🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGuard the NetCDF download and plot cells when no successful execution exists.
When no execution group has a successful execution, cell 17 sets
outputs = []and prints the helpful message. Cell 21 then runsnetcdfs[0]on an empty list, which raisesIndexError. The notebook stops at cell 21, so the helpful message does not complete the workflow that issue#2asks for. Cell 23 also usesdsandlocal, which cell 21 never defines in this case.Suggested fix
Cell 21:
netcdfs = [o for o in outputs if o.filename.endswith(".nc")] -nc = netcdfs[0] -print("downloading", nc.filename) - -response = requests.get(nc.url, timeout=60) -response.raise_for_status() - -local = Path(tempfile.gettempdir()) / Path(nc.filename).name -local.write_bytes(response.content) - -ds = xr.open_dataset(local) -ds +if not netcdfs: + print("This diagnostic has no NetCDF output, so there is no dataset to plot.") + ds = None + local = None +else: + nc = netcdfs[0] + print("downloading", nc.filename) + + response = requests.get(nc.url, timeout=60) + response.raise_for_status() + + local = Path(tempfile.gettempdir()) / Path(nc.filename).name + local.write_bytes(response.content) + + ds = xr.open_dataset(local) + display(ds)Cell 23:
import matplotlib.pyplot as plt -plt.figure(figsize=(8, 3)) - -# Note: the "unknown" variable in this case is precipitation -ds.mean("lon")["unknown"].plot() - -plt.gca().set_title("Zonal mean precipitation") -plt.show() - -# Release the dataset, downloaded NetCDF, and figure before the notebook ends. -ds.close() -local.unlink(missing_ok=True) +if ds is None: + print("Skipping the plot because no NetCDF output is available.") +else: + plt.figure(figsize=(8, 3)) + + # Note: the "unknown" variable in this case is precipitation + ds.mean("lon")["unknown"].plot() + + plt.gca().set_title("Zonal mean precipitation") + plt.show() + + # Release the dataset, downloaded NetCDF, and figure before the notebook ends. + ds.close() + local.unlink(missing_ok=True) + free_memory()
01-ref-concepts.ipynb:287
notebooks/01-ref-concepts.ipynb:287
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHandle an empty execution-group list before indexing.
If the API returns no execution groups for
example,execution_groups[0]raisesIndexErrorbefore the new no-success message runs. Check for an empty list here and let the outputs cell show a helpful message for that case. (raw.githubusercontent.com)
-
-
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
69130788-b747-48de-ae66-2eefb1ddf48c
-
notebooks/01-ref-concepts.ipynbnotebooks/02-querying-the-api.ipynbnotebooks/03-publication-figure.ipynbnotebooks/04-local-diagnostic-run.ipynbnotebooks/05-ecs-temperature-figure.ipynbsrc/ref_tutorials/__init__.pysrc/ref_tutorials/data.pytests/test_ref_tutorials.py
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
-
Fetching one execution group at a time still timed out on CI, because the API serves few requests at once. Paging the diagnostic's series takes about five requests.
Fixes the notebooks after the REF API removed
latest_executionfrom execution groups (#2).latest_successful_executionhelper that picks the newest successful, unretracted execution fromexecutions, orNone.If a diagnostic in notebook 02 has no successful execution at all, the outputs cell prints a message but the NetCDF download cell after it still fails. I kept it that way rather than guarding every later cell, because the tutorial needs real outputs to make sense.
Adds
nbstripoutto pre-commit to drop cell execution timestamps, keeping outputs, counts and cell ids. All notebook outputs are re-executed against the live API, so 03 and 04 show output-only diffs.uv run pytest --nbmake notebookspasses for all five notebooks locally. The CI workflow had been disabled for inactivity, so it missed this API change. It is enabled again.Fixes #2