Skip to content

Issue #593: Rename "range" to "interval_range"#616

Merged
nikosbosse merged 6 commits intomainfrom
update-interval_range
Feb 13, 2024
Merged

Issue #593: Rename "range" to "interval_range"#616
nikosbosse merged 6 commits intomainfrom
update-interval_range

Conversation

@nikosbosse
Copy link
Copy Markdown
Collaborator

@nikosbosse nikosbosse commented Feb 2, 2024

Description

This PR closes #593

As mentioned in #593, we're currently using "range" to denote the range of a prediction interval. To make this clearer, this PR replaces "range" with "interval_range".

Specifically, this PR

  • is very boring
  • replaces "range" with "interval_range" both in the code and the documentation
  • renames the range format (used internally) to interval format (more consistent with what other functions are already using)
  • Renames plot_ranges() to plot_interval_ranges() - I'm thinking more and more that we should maybe just drop the function altogether for now and reintroduce it in the future
  • adds a news item

I don't know why the snapshot test is failing (the same issue appeared in #592). Locally, everything is fine.

Checklist

  • My PR is based on a package issue and I have explicitly linked it.
  • I have included the target issue or issues in the PR title as follows: issue-number: PR title
  • I have tested my changes locally.
  • I have added or updated unit tests where necessary.
  • I have updated the documentation if required.
  • I have built the package locally and run rebuilt docs using roxygen2.
  • My code follows the established coding standards and I have run lintr::lint_package() to check for style issues introduced by my changes.
  • I have added a news item linked to this PR.
  • I have reviewed CI checks for this PR and addressed them as far as I am able.

@seabbs
Copy link
Copy Markdown
Contributor

seabbs commented Feb 7, 2024

its because data.table has been updated on CRAN. if you update I think you will see the same issues.

@nikosbosse
Copy link
Copy Markdown
Collaborator Author

Ahh thanks that makes sense. Fix in #618.

@nikosbosse nikosbosse closed this Feb 8, 2024
@nikosbosse nikosbosse reopened this Feb 8, 2024
Copy link
Copy Markdown
Contributor

@seabbs seabbs left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 13, 2024

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (39c95d4) 85.88% compared to head (da79478) 85.90%.

Files Patch % Lines
R/metrics-quantile.R 77.77% 4 Missing ⚠️
R/check-inputs-scoring-functions.R 60.00% 2 Missing ⚠️
R/summarise_scores.R 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #616      +/-   ##
==========================================
+ Coverage   85.88%   85.90%   +0.02%     
==========================================
  Files          21       21              
  Lines        1771     1781      +10     
==========================================
+ Hits         1521     1530       +9     
- Misses        250      251       +1     

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

@nikosbosse nikosbosse merged commit efaaa5a into main Feb 13, 2024
@nikosbosse nikosbosse deleted the update-interval_range branch February 13, 2024 12:55
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.

Is range better to be removed from the data.frame returned by add_coverage?

3 participants