Skip to content

888: Fix get_duplicate_forecasts() deprecation message - #1232

Open
nikosbosse wants to merge 2 commits into
mainfrom
fix-get-duplicate-forecasts-type
Open

nikosbosse wants to merge 2 commits into
mainfrom
fix-get-duplicate-forecasts-type

Conversation

@nikosbosse

@nikosbosse nikosbosse commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator

CLAUDE: Fixes the deprecation warning added in #1148 (#888), before it ships in 2.3.0.

Problem

Calling get_duplicate_forecasts() on a plain data.frame without type warned:

The `type` argument of `get_duplicate_forecasts()` is deprecated as of scoringutils 2.2.0.
ℹ Pass `type` (e.g. "quantile", "sample") to detect type-specific duplicates on plain data.frames.

That says the opposite of what is meant. The type argument isn't deprecated; calling without it is. The details line then tells users to pass type, which contradicts the first line. The version is also wrong: the change ships in 2.3.0.

Fix

The message now reads:

Calling `get_duplicate_forecasts()` on a data.frame without specifying `type`
was deprecated in scoringutils 2.3.0.
ℹ Pass `type` (e.g. "quantile", "sample") to detect type-specific duplicates on
  plain data.frames.
  • The custom text uses lifecycle::deprecate_warn(what = I(...)), which needs lifecycle ≥ 1.0.2 (from 2022). DESCRIPTION now requires that version.
  • The deprecation test now checks the wording and the version.

The argument order stays as #1148 has it: data, forecast_unit, type, counts. Earlier revisions of this PR moved type and added a check on its value; after discussion we dropped that in favour of this minimal fix.

🤖 Generated with Claude Code

@codecov

codecov Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.18%. Comparing base (aa3d834) to head (f1c5819).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1232   +/-   ##
=======================================
  Coverage   99.18%   99.18%           
=======================================
  Files          41       41           
  Lines        2331     2333    +2     
=======================================
+ Hits         2312     2314    +2     
  Misses         19       19           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nikosbosse nikosbosse left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm.

I slightly prefer the order of types, counts vs. counts, types, but not breaking code is also nice

@nikosbosse nikosbosse changed the title 888: Keep counts as third argument of get_duplicate_forecasts() and fix deprecation message 888: Make type the second argument of get_duplicate_forecasts() and fix deprecation message Sep 24, 2026
The message said the type argument was deprecated as of 2.2.0. What is
deprecated is calling on a plain data.frame without type, as of 2.3.0.
Custom text via I() needs lifecycle >= 1.0.2.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@nikosbosse
nikosbosse force-pushed the fix-get-duplicate-forecasts-type branch from dd34fd5 to c04b47f Compare September 25, 2026 17:19
@nikosbosse nikosbosse changed the title 888: Make type the second argument of get_duplicate_forecasts() and fix deprecation message 888: Fix get_duplicate_forecasts() deprecation message Sep 25, 2026
Fixes namespace_linter in lint-changed-files.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

This branch has not been deployed

No deployments
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