Skip to content

[Hotfix] change the default era to NULL in multiple functions#236

Open
Schiano-NOAA wants to merge 4 commits intomainfrom
hotfix-default-era
Open

[Hotfix] change the default era to NULL in multiple functions#236
Schiano-NOAA wants to merge 4 commits intomainfrom
hotfix-default-era

Conversation

@Schiano-NOAA
Copy link
Copy Markdown
Collaborator

Setting default era to NULL instead of "time" so nothing is unnecessarily filtered by default

@github-actions
Copy link
Copy Markdown
Contributor

New version checklist

  • Package version in DESCRIPTION has been updated
  • Release notes have been drafted/published
  • Cheatsheet content has been updated (if applicable)
  • Cheatsheet version has been updated

@Schiano-NOAA Schiano-NOAA marked this pull request as ready for review April 27, 2026 20:54
@Schiano-NOAA Schiano-NOAA changed the base branch from main to dev April 27, 2026 20:55
@Schiano-NOAA Schiano-NOAA changed the base branch from dev to main April 27, 2026 20:58
recruitment_label = "mt",
interactive = TRUE,
# era = "time",
era = "time",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should this be NULL, just like for the other plotting functions?

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.

No I had set it NULL but it made the tests fail so something is wack when it's set to NULL

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do you know what's unexpectedly happening? It's strange that everything else works when this argument is set consistently, except for this one

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.

I think in this case, it needs era = "time" because it has that in the values? But I would have to look into it to be sure

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.

2 participants