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

supress readr output from 'write_package' #120

Closed
mpadge opened this issue Jan 3, 2023 · 15 comments
Closed

supress readr output from 'write_package' #120

mpadge opened this issue Jan 3, 2023 · 15 comments

Comments

@mpadge
Copy link
Contributor

mpadge commented Jan 3, 2023

Current behaviour produces by default the output from readr (which can't be reprex'ed here, because it's via cat, no ignored by reprex). It'd be good to have a quiet option in write_package which simply did this:

if (quiet) {
  op <- options(readr.show_progress = FALSE)
}
# ... write package code
if (quiet){
  options(op)
}
@peterdesmet
Copy link
Member

Thanks for the suggestion. Would you suggest something similar for read_resource() which shows e.g. readr progress bars by default?

@mpadge
Copy link
Contributor Author

mpadge commented Jan 3, 2023

Thanks for the quick response @peterdesmet. Note a more general context for my interest in frictionless: I'm leading a small-ish rOpenSci project to develop "deposits", for which we are going to start using frictionless as a standard metadata template. As long as this comment of yours remains applicable, particularly in regard to permitting custom properties, it should be implemented relatively soon. I'll email you soon to give a more general description of our intentions with regard to frictionless. Great work!

@peterdesmet
Copy link
Member

Excellent! Great to hear that you plan to adopt frictionless for deposits. Some personal thoughts:

  1. I think datapackage.json is excellent to describe the data files of a (Zenodo) deposit, their relationships and their fields. I have done so for a number of datasets.

  2. Adding properties to datapackage.json can indeed be done with the append() functionality I suggested at in this comment. I'm not sure it warrants a dedicated function?

  3. This frictionless package only describes the name, type and enum of the fields, but that can be extended with description, format etc. I have done so in a R package for tracking data that builds upon frictionless: https://github.com/inbo/movepub/blob/860521adb200442843cb8607d59c121e7f67eb0c/R/add_resource.R#L43-L76 Again, there is no dedicated function in frictionless to add properties for each field. Do you think that is needed?

  4. I think datapackage.json can be tricky to describe the general metadata of a deposit. It can definitely hold that metadata (and has the possibility to extend it with e.g. related identifiers). But that info also needs to be repeated in the metadata of the deposit itself. The latter can be edited without creating a new deposit, while the datapackage.json (being part of the data) cannot. This can lead to discrepancies between the two.

  5. Note that frictionless-py v5 now has functionality to deposit on Zenodo (https://framework.frictionlessdata.io/docs/portals/zenodo.html). I haven't used it myself, but I am more a fan of separating that functionality over two packages as you suggest with frictionless vs deposits R packages.

@PietrH
Copy link
Member

PietrH commented Jan 27, 2023

For both write_package() and read_resource():

  • should the default behaviour be to show a progress bar, or not?
  • should there be any other checks? readr also checks for an interactive session or notebook chunks.

@peterdesmet
Copy link
Member

  1. Should the default behaviour be to show a progress bar, or not?

Yes, show by default, as that is the default readr behavior. Silence with explicit quiet=TRUE

  1. Should there be any other checks ...

I wasn't aware, but I would leave those enabled by default

@PietrH
Copy link
Member

PietrH commented Jan 31, 2023

I did a bit more digging and I found a few more cases where progress bars could pop up:

Of the readr functions that use the progress argument, add_resource() + read_resource() call readr::read_delim() and also write_resource() calls readr::write_csv()

Or in other words, quiet should be implemented in:

  • write_resource()
  • add_resource()
  • read_resource()

I'd like to have a go at implementing a quiet argument, probably either calling readr::show_progress() or a similar implementation:

  isTRUE(getOption("readr.show_progress")) &&
    rlang::is_interactive() &&
    !isTRUE(getOption("rstudio.notebook.executing"))

@mpadge
Copy link
Contributor Author

mpadge commented Jan 31, 2023

@peterdesmet Thanks for considering this now admittedly naive request so seriously. The ensuing discussion via (private) rOpenSci slack led, .among other things, to tidyverse/design#42, which raises some good arguments for considering approaches different to my suggestion of a function-level parameter. In particular, i find the argument for package-level control compelling, and it is indeed then very easy to implement some kind of options.frictionless_verbosiry=<level> to feed on through to any message-issuing parts of the frictionless workflow.

So @PietrH I'd suggest a little more discussion here on preferred approach(es) before impelenting anything 😃

@PietrH
Copy link
Member

PietrH commented Jan 31, 2023

@mpadge I understand, I'll hold off for now, I'm certainly invested so I'll contribute to the discussion where I can.

@peterdesmet
Copy link
Member

Thanks to reply here as well @mpadge. I'm happy to support what is recommended by the rOpenSci dev guide.

@mpadge
Copy link
Contributor Author

mpadge commented Jan 31, 2023

And we'd be very happy to have you help us by expressing any opinions via the link shown above 😉 . Failing that, we've scheduled a chunk of Dev Guide work for Feb 9th, and that might hopefully yield some concrete recommendations.

@peterdesmet
Copy link
Member

@mpadge any news on community-wide recommendations to silence messages?

@mpadge
Copy link
Contributor Author

mpadge commented Mar 28, 2023

@mpadge any news on community-wide recommendations to silence messages?

Nope, alas. Still waiting for the tidyverse/design issue referenced above to be resolved.

@peterdesmet
Copy link
Member

Community-wide recommendations on silencing are now published by @mpadge and @maelle at https://ropensci.org/blog/2024/02/06/verbosity-control-packages/ 🎉

@mpadge I will update read_package() and read_resource() that it can be silenced. I will likely also set a frequency for read_package(), so it only shows the message when a specific dataset (based on its DOI) is loaded for the first time.

@PietrH
Copy link
Member

PietrH commented Feb 8, 2024 via email

@peterdesmet
Copy link
Member

@mpadge the upcoming version of frictionless no longer returns a message (on usage rights) in read_package() #121.

write_package() still does return a message for each file it is downloading. I prefer to keep those, to inform the user what is happening. We could extend those to progress bars, but I don't see that as a high priority.

The main change is that these messages are now written with cli::cli_inform(class = "frictionless_message_file_downloading") and can be silenced as per the recommendations in the blog post (see reprex below). Note that only a global or local silence option is supported, not a package-level one, since I didn not create wrappers around the cli_inform() to support local_options(mypackage.verbose = "verbose"). If that is something you need, please open a new issue.

library(frictionless)
library(magrittr)
p1 <- read_package("https://zenodo.org/records/5653311/files/datapackage.json")
write_package(p1)
#> Downloading file from
#> 'https://zenodo.org/records/5653311/files/O_ASSEN-reference-data.csv'.
#> Downloading file from
#> 'https://zenodo.org/records/5653311/files/O_ASSEN-gps-2018.csv.gz'.
#> Downloading file from
#> 'https://zenodo.org/records/5653311/files/O_ASSEN-gps-2019.csv.gz'.
#> Downloading file from
#> 'https://zenodo.org/records/5653311/files/O_ASSEN-acceleration-2018.csv.gz'.
#> Downloading file from
#> 'https://zenodo.org/records/5653311/files/O_ASSEN-acceleration-2019.csv.gz'.

rlang::local_options(rlib_message_verbosity = "quiet")
p2 <- read_package("https://zenodo.org/records/10055494/files/datapackage.json")
write_package(p2)

rlang::local_options(rlib_message_verbosity = "verbose")
p3 <- read_package("https://zenodo.org/records/10054818/files/datapackage.json")
write_package(p3)
#> Downloading file from
#> 'https://zenodo.org/records/10054818/files/MEDGULL_ANTWERPEN-reference-data.csv'.
#> Downloading file from
#> 'https://zenodo.org/records/10054818/files/MEDGULL_ANTWERPEN-gps-2021.csv.gz'.
#> Downloading file from
#> 'https://zenodo.org/records/10054818/files/MEDGULL_ANTWERPEN-gps-2022.csv.gz'.

Created on 2024-03-27 with reprex v2.1.0

@peterdesmet peterdesmet added this to the 1.1.0 milestone Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants