Skip to content

Report time dimension in geolocation output and add multi-time integration tests#131

Merged
eeholmes merged 3 commits intomainfrom
copilot/add-nearest-time-implementation
Mar 18, 2026
Merged

Report time dimension in geolocation output and add multi-time integration tests#131
eeholmes merged 3 commits intomainfrom
copilot/add-nearest-time-implementation

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 18, 2026

  • Understand the existing implementation (time dim detection and nearest time selection already exist in engine.py)
  • Update _geoloc_description() in _open_method.py to include time dim when present
  • Update open_dataset() in plan.py to detect time dim and pass it to _geoloc_description()
  • Add tests for time dim reporting in open_dataset()
  • Add integration tests using multi_time.nc fixture
  • Run code review and address feedback
  • Run CodeQL security check
  • Merge main (PR Rename spatial methods; fix opened_batch mutation bug; add xoak-haversine #135): spatial method renames (ndpoint→kdtree, xoak→xoak-kdtree, xoak-haversine added), opened_batch mutation fix
Original prompt

This section details on the original issue you should resolve

<issue_title>Add an implementation of time method = "nearest" that finds the files that are closest in time.</issue_title>
<issue_description>Task: Implement finding nearest time if a xarray Dataset has time dim and multiple time points. If there is time dim in xarray dataset but only 1 time, then matching to nearest time is not necessary (skip).

Here is a sample netcdf with 366 days for testing
https://github.com/fish-pace/point-collocation/tree/main/examples/fixtures/multi_time.nc

Here is a notebook that shows some sample points and finding nearest lat/lon/time
https://github.com/fish-pace/point-collocation/blob/main/examples/multi_time.ipynb

Background: In some cases, a granule will have multiple times. For example, a yearly granule with 12 months or 365 days. A lat/lon/time will match to that granule, but the time needs to be matched to nearest time.

Notes:

  • time should always be 1D. but might not be uniform.
  • although unusual, it is possible that 2D lat/lon would have multiple times.
  • if there is only 1 time, then matching to nearest time is not necessary
  • cf_xarray should be used to find the time dim, and a reasonable fallback used.
  • report time dim with geolocation in open_dataset() and anywhere else that geolocation is reported. Do not report time dim if the granules do not have a time dim</issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: eeholmes <2545978+eeholmes@users.noreply.github.com>
Copilot AI changed the title [WIP] Add implementation of time method nearest for file matching Report time dimension in geolocation output and add multi-time integration tests Mar 18, 2026
Copilot AI requested a review from eeholmes March 18, 2026 01:12
@eeholmes
Copy link
Copy Markdown
Member

@copilot
I just did a merge into main
#135

It hopefully will not affect the code in the current PR. But review the changes from PR 135, pull into your branch and make any necessary updates.

…on bug, add xoak-haversine

Co-authored-by: eeholmes <2545978+eeholmes@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 18, 2026

@copilot
I just did a merge into main
#135
...

Pulled in PR #135 cleanly — automatic merge, no conflicts. All 304 tests pass (commit 5e3ad4a). The changes from PR #135 (spatial method renames: ndpointkdtree, xoakxoak-kdtree, new xoak-haversine, and the opened_batch mutation bugfix) don't touch _open_method.py or plan.py, so there's no overlap with this PR's changes.

@eeholmes eeholmes marked this pull request as ready for review March 18, 2026 20:33
@eeholmes eeholmes merged commit c602b68 into main Mar 18, 2026
@eeholmes eeholmes deleted the copilot/add-nearest-time-implementation branch March 18, 2026 20:34
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.

Add an implementation of time method = "nearest" that finds the files that are closest in time.

2 participants