test: reach 99.9% coverage for nodates feature - #278
Merged
Conversation
Add integration tests targeting the 16 previously uncovered lines: - detect_trends:40 — integer date_col path - plot_pytrendy:172-178,182 — prev fill (string + integer branches) - plot_pytrendy:210-216 — next noise fill (string + integer + date branches) - abrupt_shaving:93 and artifact_cleanup:115 tested via detect_trends inputs Coverage improved from 16 to 2 missed lines (99% -> 99.8%). plot_pytrendy.py now at 100%.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
The out-of-range guard at line 93 (new_start < df.index[0]) was added in PR #205 when the index logic changed from datetime to generic. However, the pipeline never produces abrupt segments at df.index[0] (the Savitzky-Golay window requirement prevents it), making this dead code. On develop, the equivalent datetime path has no guard and works fine.
Contributor
|
📚 Docs preview removed The docs preview for this PR has been cleaned up. |
RussellSB
changed the title
RussellSB
commented
Aug 4, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
RussellSB
marked this pull request as draft
August 4, 2026 18:31
…_cleanup The nodates branch reindexes DataFrames to contiguous integers before the pipeline runs. All segments store integer positions. But artifact_cleanup.py still used pd.to_datetime(), .strftime(), and pd.Timedelta() — leftover from the old datetime code. Replaced all datetime operations with plain integer arithmetic (+1, -1) matching the pattern used by abrupt_shaving, expand_contract, and update_neighbours. Also updated fill_in_flats to store integer values directly instead of formatting as date strings.
Revert cosmetic changes (dict syntax, whitespace, docstring) that were accidentally included in the previous commit. The functional code is identical to origin/nodates.
RussellSB
marked this pull request as ready for review
August 5, 2026 07:32
…l branches Add tests that call plot_pytrendy directly with crafted segments to exercise the prev fill (lines 172-178, 182) and next noise fill (lines 210-216) branches. Uses custom data with dip/spike patterns to trigger invalid displacement conditions. Coverage: plot_pytrendy.py 92% -> 100%, total 99.9% (1 line remaining) TODO: move tests to tests_plotting/edgecases/ when restructuring.
…t tests - Rename TestPlotPrevFill -> TestPrevFill, TestPlotNextNoiseFill -> TestNextNoiseFill (they assert segments via detect_trends, not plots) - Convert 5 direct plot tests to @pytest.mark.mpl_image_compare with baseline PNGs saved to tests/tests_plotting/edgecases/ - Tests call detect_trends then craft segments to trigger the prev fill and next noise fill branches in plot_pytrendy (lines 172-178, 182, 210-216) - TODO left to redo with more realistic synthetic scenarios later plot_pytrendy.py now at 100% coverage; total 99% (1 line remaining: artifact_cleanup:115, pre-existing dead code tracked in #279)
ChrisMarsden833
approved these changes
Aug 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Improves test coverage from 95% to 99.9% by adding integration tests for previously uncovered lines in PR #205. Also removes one unreachable dead code guard added defensively in PR #205.
Commits
results.segments[0]plot_pytrendydirectly with crafted segments to exercise lines 172-178, 182, 210-216What was covered
detect_trends.pytest_integer_date_col_returns_integerdate_colplot_pytrendy.pytest_string_prev_fill_directplot_pytrendywith string index, crafted Flat→Up with invalid displacementplot_pytrendy.pytest_integer_prev_fill_directplot_pytrendy.pytest_date_next_noise_fill_directplot_pytrendywith date index, crafted Down→Noise with invalid displacementplot_pytrendy.pytest_string_next_noise_fill_directplot_pytrendy.pytest_integer_next_noise_fill_directWhat was removed
abrupt_shaving.pycontinuefor out-of-range boundsdf.index[0]. On develop, the equivalent datetime path has no guard and works fine. See commit8afd742.Remaining uncovered (1 line)
artifact_cleanup.pyhas_partial_overlap_prevtrend-after-flat branchNotes
All tests are integration tests through
detect_trendsor the publicplot_pytrendyAPINo internal functions called directly
test_coverage.pynaming follows Chris's existing convention. TODO: move direct plot tests totests_plotting/edgecases/when restructuring.Note on direct plot tests (
TestPlotPrevFillDirect,TestPlotNextNoiseFillDirect): the hand-crafted segment lists are a bit contrived — they force specific displacement conditions that don't occur naturally in the existing datasets. They serve to reach the previously uncovered branches inplot_pytrendy. A TODO is left in the test file to redo these with more realistic synthetic scenarios when a natural dataset produces these patterns.Related issues