Skip to content

Hotfixes: fix check_value() error messaging + remove duplicate receiver_ids + write_dwc(rights_holder = NULL) sql bug #357

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

PietrH
Copy link
Member

@PietrH PietrH commented Apr 22, 2025

Hotfixes

check_value() provides an error when a value is missing from a reference vector. However, currently, when the reference vector contains NA, and the assertion fails (which works correctly), the resulting message doesn't list the contents of the reference vector but replaces it by NA.

  • Add to NEWS
  • Update version number: patch
  • run tests with db access
  • build_site : for upping version number
  • create tag + release after merge to main

AI Summary

Bug Fixes and Improvements:

  • Fixed an issue in check_value() where error messages failed to format correctly when NA values were present in the reference list. The logic for removing NA values was corrected.
  • Resolved a bug in list_receiver_ids() where NA values were included in the results. The function now explicitly filters out NA values.
  • Addressed a problem in write_dwc() where the function would fail if rights_holder was not provided. A default value of NA_character_ is now assigned when rights_holder is missing.

Test Enhancements:

  • Added new tests for check_value() to verify it handles NA values in the reference list and produces appropriate error messages.
  • Enhanced tests for list_receiver_ids() to ensure it returns a unique, non-NA character vector and includes known values.
  • Introduced tests for write_dwc() to validate the behavior of the rights_holder parameter, including cases where it is explicitly set or left as default.

Documentation and Metadata:

  • Updated the package version to 2.2.2 in DESCRIPTION to reflect the new changes.
  • Added a changelog entry in NEWS.md summarizing the fixes and improvements in this release.

PietrH added a commit to inbo/etnservice that referenced this pull request Apr 22, 2025
@PietrH
Copy link
Member Author

PietrH commented Apr 22, 2025

Found bug : #358

@PietrH PietrH changed the title Hotfix: fix check_value() error messaging Hotfixes: fix check_value() error messaging + remove duplicate receiver_ids` Apr 22, 2025
@PietrH PietrH linked an issue Apr 22, 2025 that may be closed by this pull request
@PietrH PietrH linked an issue Apr 22, 2025 that may be closed by this pull request
@PietrH PietrH changed the title Hotfixes: fix check_value() error messaging + remove duplicate receiver_ids` Hotfixes: fix check_value() error messaging + remove duplicate receiver_ids + write_dwc(rights_holder = NULL)` sql bug Apr 22, 2025
@PietrH PietrH changed the title Hotfixes: fix check_value() error messaging + remove duplicate receiver_ids + write_dwc(rights_holder = NULL)` sql bug Hotfixes: fix check_value() error messaging + remove duplicate receiver_ids + write_dwc(rights_holder = NULL) sql bug Apr 22, 2025
@PietrH PietrH self-assigned this Apr 22, 2025
@PietrH PietrH requested a review from peterdesmet April 22, 2025 14:31
@PietrH PietrH marked this pull request as ready for review April 22, 2025 14:31
Copy link
Member

@peterdesmet peterdesmet left a comment

Choose a reason for hiding this comment

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

Longest feature branch name ever 😉.

@@ -1,6 +1,6 @@
Package: etn
Title: Access Data from the European Tracking Network
Version: 2.2.1
Version: 2.2.2
Copy link
Member

Choose a reason for hiding this comment

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

If you bump here, don't forget to bump in CITATION.cff before you release. Same with DOI in CITATION.cff, which is tricky, since you don't know it yet. I have no solution for this.

Copy link
Member Author

Choose a reason for hiding this comment

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

I used the general, all version DOI in my citation.cff. Was that a mistake?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, didn't catch that. That works! 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

What about this action, if it triggers on release, do I still need to do anything?

https://github.com/inbo/etn/blob/main/.github/workflows/update-citation-cff.yaml

@PietrH PietrH merged commit d502d4f into main Apr 23, 2025
5 checks passed
@PietrH PietrH deleted the 356-bug-check_value-is-unable-to-handle-na-in-reference-when-failing-assertion branch April 23, 2025 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants