Skip to content

Add dark ecology support to read_vpts() - #187

Merged
bart1 merged 41 commits into
mainfrom
175-dark-ecology
Jun 11, 2026
Merged

bart1 merged 41 commits into
mainfrom
175-dark-ecology

Conversation

@PietrH

@PietrH PietrH commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Two ideas: read via arrow, or via guessing paths and vroom

TODO

  • create base equivalent to stringr::str_pad
  • Refactor to only use directory scanning, not {arrow}
  • use bioRad:read_cajun() 🌶️ instead of vroom
  • remove default arg for local dir
  • rename dateinterval argument to rounded_interval
  • rename path to directory
  • add call argument
  • add ellipsis before call
  • implement call
  • Include some files in package for testing, but add them to buildignore to exclude them from CRAN
  • skip examples if no test files are present
  • add tests
  • drop stringr dep
  • skip tests if no test files are present
  • check coverage
  • update NEWS

Ideas

  • Make arrow dep optional and fall back on vroom
  • Make fs dep optional or use base all the time
  • Allow returning as tibble to simplify return_type argument in get_vpts
  • consider duckdb over arrow? duckdbfs will not help because we don't have filesystem access.

Trouble

  • Getting a local unarchived version of dark ecology vpts profiles is non trivial, a single year exceeds the inode quota of the ext4 partition I am using
  • how to test this if you need a specific local filesystem?

@PietrH PietrH linked an issue Jun 4, 2026 that may be closed by this pull request
@PietrH PietrH self-assigned this Jun 4, 2026
@codecov

codecov Bot commented Jun 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.67%. Comparing base (0dd2fc9) to head (4dc1ada).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #187      +/-   ##
==========================================
+ Coverage   92.45%   92.67%   +0.22%     
==========================================
  Files          28       30       +2     
  Lines        2054     2185     +131     
==========================================
+ Hits         1899     2025     +126     
- Misses        155      160       +5     

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

@bart1

bart1 commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

Also consider using bioRad::read_cajun instead of vroom as it avoids having to implement the parsing

Comment thread tests/testthat/test-get_vpts_local.R Outdated
@bart1

bart1 commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

I moved all path test to get_vpts_local so they work for both functions

One thing we might think about it if for dark ecology the return_type="tibble" should work

@bart1

bart1 commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

@YohanSassi to also support reading data from different sources the setup for local data changes a bit in the release from the development version. I'm not sure if you are using it but in that case I wanted to give you a heads up

@bart1 bart1 mentioned this pull request Jun 8, 2026
24 tasks
Comment thread R/get_vpts_dark_ecology.R Outdated
@bart1

bart1 commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Note, currently if no data has been found for a radar it is omitted from the result with a warning. As a result the return value of the following call might either be a list of vpts's or a vpts. That might be some what annoying, Should we try to always return a list when multiple radars are requested with only entries for the radars where data has been found?

get_vpts(
             c("bewid","bejab"),
             as.Date("2016-2-1"),
             path = normalizePath(local_dir)
           )

@bart1
bart1 marked this pull request as ready for review June 10, 2026 12:47
@bart1
bart1 requested a review from peterdesmet June 10, 2026 12:47
@bart1

bart1 commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

@PietrH @peterdesmet I think this is pretty much done, though I have not tested with large dataset. Only this comment is still open : #187 (comment) .

It would be good to have one extra pair of eyes.

@PietrH

PietrH commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

I had a look at your changes and have no notes. Thanks for picking this up so quickly Bart!

@bart1
bart1 merged commit 869bfa0 into main Jun 11, 2026
16 of 17 checks passed
@bart1
bart1 deleted the 175-dark-ecology branch June 11, 2026 13:08
@PietrH PietrH changed the title [WIP] Add dark ecology support to read_vpts() Add dark ecology support to read_vpts() Jul 23, 2026
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.

Integrate data from the dark ecology dataset?

2 participants