Closed fkiraly closed 3 months ago
Attention: Patch coverage is 89.65517%
with 3 lines
in your changes are missing coverage. Please review.
Project coverage is 88.63%. Comparing base (
5d53806
) to head (74776b4
). Report is 70 commits behind head on main.
Files | Patch % | Lines |
---|---|---|
src/tsbootstrap/base_bootstrap.py | 88.88% | 3 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Q from meeting today: is X
and y
necessary in get_n_bootstraps
? That is, isn't this always identical to the param, n_bootstraps
? Or, is this present for compatibility reasons?
Compatibility reasons
On Fri, Apr 12, 2024, 5:18 PM Franz Király @.***> wrote:
Q from meeting today: is X and y necessary in get_n_bootstraps? That is, isn't this always identical to the param, n_bootstraps? Or, is this present for compatibility reasons?
— Reply to this email directly, view it on GitHub https://github.com/astrogilda/tsbootstrap/pull/135#issuecomment-2052544869, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFTOOHSQDN2PXPKXZRFS56DY5BFTRAVCNFSM6AAAAABGEJWXAWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANJSGU2DIOBWHE . You are receiving this because you are subscribed to this thread.Message ID: @.***>
... with ... ?
... with ... ?
sorry forgot to respond. compatibility with crossvalidation in sklearn, which has a get_cv method iirc.
This PR streamlines the extension contract for adding more estimators. There are minimal breaking changes, imo these do not require deprecation due to changes affecting only buggy interfaces.
Changes:
_bootstrap
and_get_n_bootstraps
with a clearly documented extension template. These are called frombootstrap
andget_n_bootstraps
.get_n_bootstraps
where it was missing.groups
argument is removed fromget_n_bootstraps
.