Next version #14
Next version #14
Conversation
- dhmv api support - adjusted roxygen - more assertions - version constraint - case-insensitive wcs
- dhmv api support - adjusted roxygen - more assertions - version constraint - case-insensitive wcs
+ rebased to follow the latest dev by @hansvancalster on parallel branches + incorporated review suggestions + modified `unpack_mht()`: the function will now return the tif path + additional vignette paragraph: using DHMV `v2.0.1`
Patch mht
|
@falkmielke I will ask you to review this branch, but it is more a pro-forma question since most of the stuff here is already reviewed in the other branch. The only substantial addition is a small change in the unpack_mht function to also detect geoTIFF with different type of byte order. @florisvdh would you like to review too? Or do you perhaps have something in mind to contribute? This would be a good time. |
There was a problem hiding this comment.
I could successfully install the next-version branch locally, vignettes build as expected, outcomes look fine. I have also looked at all code changes, seems good. Thank you for bringing together the new version!
For the archives: I have kept notes to better test the geoTIFF extraction (not tested now) as examples roll over my desk; I assume it worked for your cases. I also added to my todo list to create unit tests for the different wcs services, where we query known outcomes. In particular, you mentioned a strict CRS 31370 requirement, which I will double check once I understand more about that.
Since the package works, and we practically only use 31370, these extras are not urgent. But let me know if we should keep track via an issue.
KMI query failed when remote-installing `next-release` Wrapped in a tryCatch to ensure building of vignettes.
There was a problem hiding this comment.
This review (more or less) adds to the review of PR #12.
@falkmielke I've read the spatial_dhmv_query vignette. Pleasant reading, relatively easy to follow, well done! 🌞
I locally tested the get_coverage_wcs() example in the vignette, with success!
Only minor comments added.
A minor general one is my advice to put each sentence of a paragraph in another line. Hugo will stitch those lines into one paragraph in HTML, as would Pandoc. It minimizes git diffs and it will also make review comments easier to interpret afterwards. No need to implement it here as far as I'm concerned, just saying!
Don't forget to rebuild the documentation (Rd files) after making updates in the roxygen comment section.
- wrapping in trycatch is adding code which will confuse novice users
minor additions to incorporate review feedback
Branch to collect contributions for new version of the package.
Closes #13