Skip to content

fix(PEcAn.SIPNET): return empty dataframe when date filtering removes all segments (#4007)#4048

Merged
infotroph merged 3 commits into
PecanProject:developfrom
ANAMASGARD:GH-4007-segment-dataframe-empty-after-filter
Jul 16, 2026
Merged

fix(PEcAn.SIPNET): return empty dataframe when date filtering removes all segments (#4007)#4048
infotroph merged 3 commits into
PecanProject:developfrom
ANAMASGARD:GH-4007-segment-dataframe-empty-after-filter

Conversation

@ANAMASGARD

Copy link
Copy Markdown
Contributor

Description

Fixes a bug in segment_dataframe() where date filtering could leave zero valid crop-cycle segments, but the function still continued and executed segments[1, "start_date"] <- run_start on an empty dataframe. In R, that assignment creates a ghost 1-row frame with start_date set and all other columns (end_date, crop_code, site_id, pft) as NA.

That ghost row was passed to write_segment_configs(), which then tried to build segment configs with invalid dates and PFTs. The failure appeared far downstream from the actual root cause.

Change: When filtering removes all segments, return the empty dataframe immediately (silent return, consistent with the existing pre-filter check at line 135). Downstream write_segment_configs() already handles nrow(segments) == 0 by returning the unaltered job.sh.

Files changed:

  • models/sipnet/R/write_segmented_configs.R — early return after filtering
  • models/sipnet/tests/testthat/test-write_segmented_configs.R — unit + integration tests
  • CHANGELOG.md — Unreleased / Fixed entry
  • models/sipnet/NEWS.md — package news entry

Motivation and Context

Closes #4007.

This edge case was spotted by @divine7022 during review of #4006. When run$start.date falls after all crop-cycle dates in the schedule, no segments are relevant to the requested time range. The correct behavior is to run the existing unsegmented config — not to error downstream with NA-filled segment rows.

Related: #4006

Review Time Estimate

  • Within one week

Types of changes

Checklist

  • I agree that PEcAn Project may distribute my contribution under any or all of
    • the same license as the existing code,
    • and/or the BSD 3-clause license.
  • I have updated the CHANGELOG.md.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Test Result

All tests passed.
image

That means no broken ghost row — fix is working. (Below ScreenShot)
image

Ran all SIPNET tests
image

…nProject#4007)

When run start is after all crop cycles, date filtering can leave zero
rows. Return early instead of assigning to segments[1, ...], which created
a ghost row with NA columns and caused downstream segment config errors.

Signed-off-by: Gaurav Chaudhary <chaudharygaurav2004@gmail.com>

@dlebauer dlebauer left a comment

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.

Thanks for this fix - it fixes #4007 and adds new tests. Ready to merge.

I made an optional suggestion - to test for specific col names rather than that ncol > 0.

Comment thread models/sipnet/tests/testthat/test-write_segmented_configs.R Outdated

@divine7022 divine7022 left a comment

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.

lgtm!, one small thing and agree with david's inline

Comment thread models/sipnet/R/write_segmented_configs.R Outdated
@infotroph
infotroph enabled auto-merge July 16, 2026 16:51
@infotroph
infotroph added this pull request to the merge queue Jul 16, 2026
Merged via the queue into PecanProject:develop with commit 513e0a0 Jul 16, 2026
19 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

segment_dataframe: return empty if no segments after filtering

4 participants