Skip to content
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

Submit to CRAN #46

Closed
Robinlovelace opened this issue Apr 19, 2021 · 16 comments
Closed

Submit to CRAN #46

Robinlovelace opened this issue Apr 19, 2021 · 16 comments
Labels

Comments

@Robinlovelace
Copy link
Contributor

This package is very, very useful. Yet it is not very visible to users. I think getting on CRAN will make it way more useful. The fact it has external dependencies that may not be installed on CRAN servers should not be a barrier, as outlined here: r-spatial/discuss#41 (comment)

I'm happy to run some tests to see what happens after running

devtools::check_win_devel()

will also help us with our work updating Geocomputation with R to use more modern and effective packages.

@paleolimbot
Copy link
Collaborator

paleolimbot commented Apr 21, 2021

Sorry to leave this hanging for two days...I'm totally fine with a CRAN submission but I think there are a few issues that need to be addressed before any particular release (of qgisprocess) can be considered "stable". That said, I think maintaining this particular CRAN package will not be hard because it can't run any actual tests (because QGIS is not installed). I think I'd still prefer users to install this via remotes::install_github() for now because new users will get the latest fix (and #36 will be a big fix).

Happy to debate this list, but in my head these are the things that need to happen before there is any particular release of qgisprocess that can be considered "stable":

  • Use the JSON interface to qgisprocess where available
  • Fix the spaces in file path bug on Windows (upstream in processx)
  • After the JSON thing is done, add support for all qgis input types
  • stars package suport for rasters

@Robinlovelace
Copy link
Contributor Author

Robinlovelace commented Aug 20, 2021

Apologies for the slow response to me. I discussed this issue yesterday with @jannes-m and @Nowosad in relation to the 2nd edition of Geocomputation with R, which we plan to publish at some point in 2022, likely spring or summer, and it would be great if the package were on CRAN by then. Good news: we're happy to help with that effort although unsure which bits you need help with. Regarding your list, I guess the file paths one is the most important. The others I would see as 'nice to have'. My thinking is that having it on CRAN will open up this amazing package to a wide range of people and therefore encourage further development. Agree #36 looks like a big one...

@paleolimbot
Copy link
Collaborator

I agree that this should happen soon...I'm giving a talk on this at FOSS4G and I think that will be a good time to get a round of feedback, after which a CRAN submission seems in order.

One thing I didn't realize is that there is no way to use JSON as input yet. I flagged Nyall already, but this feature is important for getting things to work smoothly on Windows and to support all the input types. That said, I think I can fix the WKT2 thing by working around the .bat file (and will do so before FOSS4G). I'd also like to properly use the JSON output before FOSS4G since it's now available in almost everybody's installed QGIS version.

Something that would help me out would be to generate a bunch of minimal test cases for all the input types. Like...find simple algorithms that have an input type and write a test_that("input type XX works", {...}) block that passes. I imagine that will help you generate some opinions about how various R objects can be serialized to make the experience a bit more seamless (for example, st_bbox() objects have a pretty straightforward translation to what QGIS expects a bbox to look like). No pressure if you don't get to it!

@Robinlovelace
Copy link
Contributor Author

Something that would help me out would be to generate a bunch of minimal test cases for all the input types. Like...find simple algorithms that have an input type and write a test_that("input type XX works", {...}) block that passes. I imagine that will help you generate some opinions about how various R objects can be serialized to make the experience a bit more seamless (for example, st_bbox() objects have a pretty straightforward translation to what QGIS expects a bbox to look like). No pressure if you don't get to it!

That definitely sounds like something I can can help with and I imagine Jannes will be keen to get involved in the effort too. Worth creating a separate issue on it? Happy to be assigned on that.

@paleolimbot
Copy link
Collaborator

Use #13!

@jannes-m
Copy link
Collaborator

I am happy to help, of course! But before doing so, I plan to finally finish writing the vignette(s) (#31). I plan to do so within the next 2 to 3 weeks.

@florisvdh
Copy link
Member

florisvdh commented Feb 13, 2023

Updating Dewey's checklist:

  • Use the JSON interface to qgisprocess where available
  • Fix the spaces in file path bug on Windows (upstream in processx)
  • After the JSON thing is done, add support for all qgis input types (Complete input argument type parsing #13)
  • stars package suport for rasters

I'm not sure where we are with #13, but to be checked further; probably supporting JSON-input-only for some cases, and preferably tests are added to check the support for different QGIS inputs.

Proposing remaining steps wrt CRAN submission:

  • Internals:
    • check support for all qgis input types
    • add argument assertions in all exported functions, using assert_that()
    • check and update consistent usage of assert_qgis()
    • increase code coverage
  • Exported functions:
    • revise choice between exported vs internal functions (my feeling is that not all current exported functions are meant for direct use by users)
    • revise function name choices. E.g. 'use_json_output' sounds as a command (do that) but only informs about a config state.
  • Documentation:
    • add vignettes (#2, PR #31)
    • document functions more consistently
    • update README (introduction)
    • further improve pkgdown reference (e.g. making use of function families to create subtitles)
  • Organisational:
    • perhaps link this package with r-spatial, or flag as such / move it there
    • create NEWS.md, bump package version
    • submit to CRAN

@florisvdh
Copy link
Member

Dewey and I had a brief meeting today.

As always, further ideas are most welcome!

@florisvdh
Copy link
Member

I'm in the stage of anticipating CRAN notes before submitting, based mainly on advice in r-pkgs.org & usethis.

@paleolimbot @JanCaha @jannes-m for CRAN submission we should add a copyright holder (cph). This should be a legal entity. I propose to add my employer INBO here, since I am allowed to also invest some personnel time for maintaining and improving. Do you agree?

Consequently, INBO is also package funder (fnd) because of this personnel time. If other funders are to be added, please shoot (similar may apply to former or current employer of @paleolimbot?)

@JanCaha
Copy link
Collaborator

JanCaha commented Jul 14, 2023

Sure, no problem ;)

@jannes-m
Copy link
Collaborator

Yeah, go for it.

@paleolimbot
Copy link
Collaborator

Go for it! No need to add my employer as a copyright holder.

@florisvdh
Copy link
Member

Great, thanks for the quick responses.

@florisvdh
Copy link
Member

BTW, I have also included affiliations in DESCRIPTION for Dewey (Voltron Data) and myself.

person("Dewey", "Dunnington", , "[email protected]", role = "aut",
comment = c(ORCID = "0000-0002-9415-4582", affiliation = "Voltron Data")),
person("Floris", "Vanderhaeghe", , "[email protected]", role = c("aut", "cre"),
comment = c(ORCID = "0000-0002-6378-6229", affiliation = "Research Institute for Nature and Forest (INBO)")),

If I should add this for more people, let me know! Affiliations appear in CITATION.cff on GitHub and also in the foreseen Zenodo publication, when hovering above the author names (example).

@florisvdh
Copy link
Member

@Robinlovelace
Copy link
Contributor Author

Amazing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants