sipnet: merged segments get the literal string "NA" in `crop_code` (segments.csv)
Maintainers usually reply within 3 days
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 78/100
Research direction
Start with drop_same_pft_segments() in models/sipnet/R/write_segmented_configs.R and the segment_dataframe() behavior described in the issue. Reproduce the example with the supplied tibble, then add a focused test for merging a missing crop_code with a populated one. Done means the merged segments.csv value omits the literal "NA" while preserving the real crop code.
Written by the indexing model from the issue text.
Description
Summary
Follow-up to #4122 / #4129. drop_same_pft_segments() in models/sipnet/R/write_segmented_configs.R builds the merged crop_code with paste(.data$crop_code, collapse = " "). A missing code is pasted as the text "NA".
The first segment (the period before the first planting) always has crop_code = NA (segment_dataframe()), so when it is merged with a following segment of the same PFT, segments.csv gets a value like "NA D12".
Reproduction (on develop)
seg <- tibble::tibble(
start_date = as.Date(c("2025-01-01", "2025-01-04", "2025-01-08")),
end_date = as.Date(c("2025-01-03", "2025-01-07", "2025-01-10")),
crop_code = c(NA, "D12", "D13"),
pft = c("soil", "soil", "crop"),
segment_id = c("001", "002", "003")
)
PEcAn.SIPNET:::drop_same_pft_segments(seg)
#> first row: crop_code = "NA D12", pft = soil, 2025-01-01 to 2025-01-07
Impact
Cosmetic: crop_code is only written to segments.csv and is not used elsewhere in the package, so run results are not affected. But the file then contains a code, "NA", that is not a real crop.
Possible fix
paste(stats::na.omit(.data$crop_code), collapse = " ") (an all-missing group would give "", which could be turned back into NA_character_). I'm happy to send a small PR with a test if this is wanted.
Also
It might be worth a test through write_segment_configs() showing that a merged segment's inputs still cover its full date range.
- Dominant language
- R
- Stars
- 246
- Forks
- 322
- Avg merge
- 7d 3h
- Merged PRs (30d)
- 17
Getting set up
- Ships a Dockerfile or Docker Compose file
- Has a pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from PecanProject/pecan
-
Tests error instead of skipping when optional packages (ecmwfr, jsonvalidate) or BETY database are unavailablePossibly taken @madhavzanwar claimed this 2 days ago. Open
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
PecanProject/pecan#4179 ·
Maintainers usually reply within 3 days
-
Scheduled workflows run on contributor forks without repository guardsPossibly taken @Labreo claimed this 48 days ago. Open
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
PecanProject/pecan#4095 · 6 comments · 2 reactions ·
Maintainers usually reply within 3 days
-
Add documentation about Docker cleanupMay be free again @soniya-malviy claimed this 625 days ago, and no pull request is open. Opendocumentation Status: Stale
Difficulty 2/5 1-3 hours Newbie friendliness 62/100
PecanProject/pecan#2693 · 2 comments ·
Maintainers usually reply within 3 days
-
do_conversions() silently ignores overwrite.met / overwrite.fia / overwrite.ic for multi-site (MultiSettings) runsPossibly taken @madhavzanwar claimed this 2 days ago. Open
Difficulty 2/5 1-3 hours Newbie friendliness 25/100
PecanProject/pecan#4175 ·
Maintainers usually reply within 3 days
-
test_remote() errors on multi-line output instead of reporting the host as working or failingPossibly taken @madhavzanwar claimed this 6 days ago. Open
Difficulty 1/5 1-3 hours Newbie friendliness 25/100
PecanProject/pecan#4168 ·
Maintainers usually reply within 3 days
All issues in PecanProject/pecan
Similar issues
-
Difficulty 1/5 Under an hour Newbie friendliness 72/100
Maintainers usually reply within 1 day
-
Difficulty 1/5 Under an hour Newbie friendliness 85/100
-
make sure .Random.seed existsPossibly taken @topepo claimed this today. Open
Difficulty 2/5 1-3 hours Newbie friendliness 66/100
tidymodels/tune#1211 ·
-
Rate metrics miscompute list and tuple labelsPossibly taken @AryanRG claimed this today. Open
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
Trusted-AI/AIF360#582 ·
-
Difficulty 1/5 Under an hour Newbie friendliness 85/100