Skip to content

Fix(#39): remove NAs before type casting - #138

Merged
BBeltz1 merged 1 commit into
devfrom
fix/i39-herring-na
Aug 7, 2026
Merged

Fix(#39): remove NAs before type casting#138
BBeltz1 merged 1 commit into
devfrom
fix/i39-herring-na

Conversation

@andybeet

@andybeet andybeet commented Aug 7, 2026

Copy link
Copy Markdown
Member

Justification

This issue was previously fixed but it appears the database has introduced additional data issues. Several STOCK_AREA values are either NAs or "NA". This warnings causes GitHub workflows to fail (although we could prevent workflows from failing on a warning, i'd prefere to fix the code). These NAs have been filtered out prior to type casting to numeric. Issue 39 was reopened.

Fixes #39

Types of changes

What types of changes does this pull request introduce? Put an x in the boxes that apply.
This will inform the new release number.

  • Fix (non-breaking change which fixes a bug)
  • Feature (non-breaking change which adds or changes functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other change (if none of the other choices apply)

Reviewer instructions

Do the following on dev and this branch.

  • Connect to the database eg. channel <- dbutils::connect_to_database("server", "user")
  • get_herring_data(channel)
    The coercion warning should be fixed

Formatting

This repo contains an air.toml file that automatically formats code to a set of standards.
It is preferred that contributors and reviewers install the Air formatting tool.
Code submitted in this pull request will be automatically checked for correct formatting.

@andybeet andybeet linked an issue Aug 7, 2026 that may be closed by this pull request
@andybeet
andybeet requested a review from BBeltz1 August 7, 2026 14:09
@andybeet
andybeet marked this pull request as ready for review August 7, 2026 14:09

@BBeltz1 BBeltz1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

code looks appropriate, coercion warning has been resolved on branch

@BBeltz1
BBeltz1 merged commit 2b1b524 into dev Aug 7, 2026
10 checks passed
@BBeltz1
BBeltz1 deleted the fix/i39-herring-na branch August 7, 2026 14:48
@andybeet andybeet mentioned this pull request Aug 7, 2026
6 tasks
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.

NAs introduced in herring pull

2 participants