Skip to content

Consistently use the test sets as reference for gap_before and gap_after #45

Open
WenjieZ wants to merge 2 commits intomasterfrom
complement_mask
Open

Consistently use the test sets as reference for gap_before and gap_after #45
WenjieZ wants to merge 2 commits intomasterfrom
complement_mask

Conversation

@WenjieZ
Copy link
Copy Markdown
Owner

@WenjieZ WenjieZ commented Feb 11, 2022

There are two ways of defining a derived cross-validator. One is to redefine _iter_test_indices or _iter_test_masks (test viewpoint), and the other is to redefine _iter_train_masks or _iter_train_indices (train viewpoint).

Currently, these two methods assign different semantic meanings to the parameters gap_before and gap_after. The test viewpoint uses the test sets as the reference:

train    gap_before    test    gap_after    train

The train viewpoint uses the training sets as the reference:

test    gap_before    train    gap_after    test

This diverged behavior is not intended inappropriate. The package should insist on the test viewpoint, and hence this PR. It will be enforced in v0.2.

I don't think this issue has touched any users, for the derived classes in this package use _iter_test_indices exclusively (test viewpoint). No users have reported this issue either. If you suspect that you have been affected by it, please reply to this PR.

@WenjieZ WenjieZ added this to the v0.2 milestone Feb 11, 2022
@WenjieZ WenjieZ changed the title Switch gap_before and gap_after for _iter_complement_mask Consistently use the test sets as reference for gap_before and gap_after Feb 11, 2022
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 11, 2022

Codecov Report

Merging #45 (fb26eda) into master (2abbc3d) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #45   +/-   ##
=======================================
  Coverage   97.51%   97.51%           
=======================================
  Files           3        3           
  Lines         643      643           
=======================================
  Hits          627      627           
  Misses         16       16           
Impacted Files Coverage Δ
tscv/_split.py 93.77% <100.00%> (ø)
tscv/tests/test_split.py 99.74% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2abbc3d...fb26eda. Read the comment docs.

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.

1 participant