Skip to content

cat, file-info, get-bbox, set-bbox, sort, merge#3

Draft
e-kotov wants to merge 31 commits into
ipea:mainfrom
e-kotov:main
Draft

cat, file-info, get-bbox, set-bbox, sort, merge#3
e-kotov wants to merge 31 commits into
ipea:mainfrom
e-kotov:main

Conversation

@e-kotov

@e-kotov e-kotov commented May 1, 2025

Copy link
Copy Markdown
Contributor

Hi @dhersz,

I'm currently working on adding a few new functions to wrap more of the osmium functionality. It all started with cat, since I wanted to glue together a few PBF files for r5r. As it turned out, designing cat properly led me to implement a few additional utilities.

So far, I’ve added:

  • osm_cat
  • osm_file_info
  • osm_file_info_variables
  • osm_get_bbox
  • osm_set_bbox
  • osm_sort

This is still a work in progress — I’m finishing up the tests and double-checking the documentation — but overall, things are already functioning well.

I also wanted to open up a conversation about potentially renaming some existing functions in the package to avoid namespace conflicts down the line. I'm following the rOpenSci guide on naming conventions and would like to suggest introducing a consistent prefix for our functions.

You can see some examples in this draft PR. Functions like extract could easily exist in many other packages, and they don’t clearly signal their connection to rosmium. My proposal would be to rename functions such as extract, time_filter, tags_filter, and show_content to something like osm_extract or rosm_extract. I’m happy to follow whatever prefix you prefer — just let me know and I can update the names in the new functions I’m contributing as well.

After renaming we could re-create old function names for existing ones to wrap the new renamed ones, but mark them as deprecated and issue a warning (e.g. like here) and keep them in the package for a few years to let everyone transition to the new function names.

Of course, this is just a suggestion. If you’d prefer not to go in this direction, I can drop the prefix from the new functions to match the current style.

This PR is still a draft — mainly to kick off the discussion.

@codecov

codecov Bot commented May 1, 2025

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 60.00000% with 272 lines in your changes missing coverage. Please review.

Project coverage is 71.59%. Comparing base (5531838) to head (7888a96).

Files with missing lines Patch % Lines
R/merge.R 0.00% 118 Missing ⚠️
R/cat.R 55.30% 59 Missing ⚠️
R/set_bbox.R 0.00% 56 Missing ⚠️
R/file_info.R 81.44% 18 Missing ⚠️
R/sort.R 78.12% 14 Missing ⚠️
R/get_bbox.R 92.10% 6 Missing ⚠️
R/check_refs.R 99.24% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main       #3       +/-   ##
===========================================
- Coverage   94.28%   71.59%   -22.70%     
===========================================
  Files           6       13        +7     
  Lines         350     1028      +678     
===========================================
+ Hits          330      736      +406     
- Misses         20      292      +272     

☔ 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.

@e-kotov e-kotov changed the title functions for cat, file_info, get and set bboxes, and sort cat, file-info, get-bbox, set-bbox, sort, merge May 6, 2025
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.

1 participant