Skip to content

Feature/issue 138 migrate to xarray.DataTree#281

Merged
ank1m merged 27 commits into
developfrom
feature/issue-138-migrate-to-DataTree
Jul 2, 2025
Merged

Feature/issue 138 migrate to xarray.DataTree#281
ank1m merged 27 commits into
developfrom
feature/issue-138-migrate-to-DataTree

Conversation

@ank1m
Copy link
Copy Markdown
Collaborator

@ank1m ank1m commented Jun 29, 2025

GitHub Issue: #138

Description

Recent l2ss-py update to 3.0.0 with newly released xarray.DataTree extension duplicates dimensions inside the netCDF groups. These duplicated dimensions break stitchee algorithm which flattens them into independent variables. Migrating stitchee logic to xarray.DataTree will resolve the issue.

Local test steps

  • replaced manual flattening and regrouping methods with xarray.DataTree methods.
  • deployed updated stitchee image in local Harmony
  • ran local SAMBAH requests for TEMPO collections

Overview of integration done

  • successful requests in local Harmony environment
    Screenshot_20250630_085949

PR Acceptance Checklist

  • Unit tests added/updated and passing.
  • Integration testing
  • CHANGELOG.md updated
  • Documentation updated (if needed).

📚 Documentation preview 📚: https://stitchee--281.org.readthedocs.build/en/281/

@ank1m ank1m marked this pull request as ready for review June 30, 2025 13:03
@ank1m ank1m requested a review from danielfromearth June 30, 2025 13:03
@danielfromearth danielfromearth added the enhancement New feature or request label Jun 30, 2025
Comment thread concatenator/stitchee.py
Comment thread concatenator/stitchee.py
Comment thread concatenator/stitchee.py
try:
# Instead of "with nc.Dataset() as" inside the loop, we use a context manager stack.
# This way all files are cleanly closed outside the loop.
with ExitStack() as context_stack:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think it's actually considered best practice use a context manager for xarray Dataset/Datatree objects too. Perhaps we should create a new issue to implement them?
See:

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jul 1, 2025

Codecov Report

Attention: Patch coverage is 88.12785% with 26 lines in your changes missing coverage. Please review.

Project coverage is 89.97%. Comparing base (9ba58f4) to head (6a159ad).
Report is 33 commits behind head on develop.

Files with missing lines Patch % Lines
concatenator/run_stitchee.py 76.74% 10 Missing ⚠️
concatenator/file_ops.py 87.67% 9 Missing ⚠️
concatenator/stitchee.py 94.52% 4 Missing ⚠️
concatenator/history_handling.py 89.65% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #281      +/-   ##
===========================================
+ Coverage    85.95%   89.97%   +4.02%     
===========================================
  Files           11        9       -2     
  Lines          648      449     -199     
===========================================
- Hits           557      404     -153     
+ Misses          91       45      -46     
Flag Coverage Δ
integration 29.17% <48.85%> (?)
unittests 89.97% <88.12%> (+4.02%) ⬆️

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

☔ 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.

@danielfromearth
Copy link
Copy Markdown
Collaborator

@ank1m, think I'm done adding commits, if you want to confirm again with your own tests before merging!

@ank1m ank1m merged commit eb42ae4 into develop Jul 2, 2025
6 checks passed
@ank1m ank1m deleted the feature/issue-138-migrate-to-DataTree branch July 7, 2025 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants