8000 Allow media-based observations by sannegovaert · Pull Request #172 · inbo/camtrapdp · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Allow media-based observations #172

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.

Sign up for GitHub

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

Jump to bottom
Open
8000 wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Allow media-based observations #172

wants to merge 12 commits into from

Conversation

sannegovaert
Copy link
Member

fix #168

@sannegovaert sannegovaert added this to the Version 0.4.0 milestone May 23, 2025
@codecov Codecov
Copy link
codecov bot commented May 23, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (b85a5a1) to head (d0802a5).

Additional details and impacted files 
@@            Coverage Diff            @@
##              main      #172   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           27        27           
  Lines         1204      1212    +8     
=========================================
+ Hits          1204      1212    +8

☔ View full report in Codecov by Sentry.
📢 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.

@sannegovaert sannegovaert requested a review from peterdesmet May 23, 2025 12:30
@sannegovaert sannegovaert marked this pull request as ready for review May 23, 2025 12:52
peterdesmet
peterdesmet requested changes May 27, 2025
View reviewed changes
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.

Nice! I've already updated write_eml() to use the same filtering (needed for scopes in metadata). @sannegovaert I think we need one more thing though: when using observation_level == "media" we need to create our multimedia.csv differently. Currently it has:

  dplyr::select(-"mediaID") %>%
  dplyr::left_join(
    media,
    by = c("deploymentID", "eventID"),
    relationship = "many-to-many" # Silence warning
  )

So it still joins on eventID while we should use mediaID in this case. So:

  • Update how the join is made depending on observation_level
  • Extend new test to also expect a multimedia_media_based.csv with an expected number of records. Currently the multimedia.csv has too many records.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peterdesmet peterdesmet peterdesmet requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Version 0.4.0
Development

Successfully merging this pull request may close these issues.

Allow media-based observations in write_dwc()
2 participants
@sannegovaert @peterdesmet
0