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

split off download functionality for use_course()? #573

Closed
beanumber opened this issue Jan 16, 2019 · 11 comments
Closed

split off download functionality for use_course()? #573

beanumber opened this issue Jan 16, 2019 · 11 comments

Comments

@beanumber
Copy link

@beanumber beanumber commented Jan 16, 2019

I see the value of use_course(), but I sometimes want to download a ZIP file that is not an RStudio project, and use it. For example, a ZIP file that lives somewhere online that contains some shapefiles that I want to use.

So how would you feel about a PR that includes use_zip(), which is just like use_course() except that it returns an fs_path to the unzipped directory? use_course() could would then call use_zip() to maintain its current functionality.

@jennybc
Copy link
Member

@jennybc jennybc commented Jan 16, 2019

I see the value of use_course(), but I sometimes want to download a ZIP file that is not an RStudio project

This already works. Just use it on a ZIP archive that does not contain an Rproj file.

For example, try this: use_course("rstd.io/wtf-explore-libraries")

But I suspect we are misunderstanding each other.

@hadley hadley closed this Mar 17, 2019
@beanumber
Copy link
Author

@beanumber beanumber commented May 31, 2019

OK. But indulge me by considering three things:

  1. As per #304 , use_course() has a narrow scope, so for example this doesn't work:
usethis::use_course("https://www.fueleconomy.gov/feg/epadata/16data.zip")
#> ✔ Downloading 'https://www.fueleconomy.gov/feg/epadata/16data.zip'
#> 
Downloaded: 0.02 MB  (2%)
Downloaded: 0.02 MB  (2%)
Downloaded: 0.03 MB  (3%)
...
Downloaded: 0.93 MB  (100%)
#> Error: Download does not have MIME type 'application/zip'.
#> Instead it's 'application/x-zip-compressed'.

Created on 2019-05-31 by the reprex package (v0.3.0)

  1. I find the forced interactivity to be annoying most of the time. I understand why it's there, but can it be optional? Can it take an interactive argument to turn this off?

  2. Why is the function called use_course() if I'm using it for something that has nothing to do with courses?

All I'm proposing would be to expose use_zip() as a more general-purpose function (with a fix for (1)), and then use_course() could continue to work as it currently does.

@jennybc
Copy link
Member

@jennybc jennybc commented May 31, 2019

Is this true? What you like about use_course() and want to extract into use_zip() is a nice download function that:

  • Figures out a reasonable file name (vs. making you do that)
  • Unpacks the ZIP archive in a sane way (vs. the typical result that often results in 1 more or 1 less level of nesting than you wanted)

@beanumber
Copy link
Author

@beanumber beanumber commented Jun 1, 2019

Yes. And returns the fs_path to the resulting directory.

So instead of:

src <- "https://www.fueleconomy.gov/feg/epadata/16data.zip"

download.file(src, destfile = "fueleconomy.zip")
unzip("fueleconomy.zip", exdir = "fueleconomy/")
lcl <- fs::path("fueleconomy/")

We'd just have:

src <- "https://www.fueleconomy.gov/feg/epadata/16data.zip"
lcl <- usethis::use_zip(src)

@jennybc jennybc reopened this Jun 2, 2019
@jennybc jennybc closed this in a84be82 Jun 2, 2019
@jennybc
Copy link
Member

@jennybc jennybc commented Jun 2, 2019

OK see what you think of use_zip(). I have also been using use_course() just to get sane ZIP download + unpack behaviour, so I see your point.

@beanumber
Copy link
Author

@beanumber beanumber commented Jun 3, 2019

Amazing!

The docs say: "doesn't insist on interactive confirmation.", but tidy_unzip() still wants to confirm the deletion of the ZIP file.

@beanumber
Copy link
Author

@beanumber beanumber commented Jun 3, 2019

And thank you!

@jennybc
Copy link
Member

@jennybc jennybc commented Jun 3, 2019

still wants to confirm the deletion of the ZIP file.

Yeah I was referring to interaction around destdir. What are your thoughts around deleting the ZIP file? I can't very well delete by default, but deleting is so nice. I think I'm OK with being asked this in an interactive setting.

@beanumber
Copy link
Author

@beanumber beanumber commented Jun 3, 2019

Could there be a delete_zip argument that defaults to "ask" but will be quiet if it's set to TRUE or FALSE?

jennybc added a commit that referenced this issue Jun 4, 2019
Relates to #573
@jennybc
Copy link
Member

@jennybc jennybc commented Jun 4, 2019

OK now you can do use_zip("r-lib/rematch2", cleanup = TRUE).

@beanumber
Copy link
Author

@beanumber beanumber commented Jun 4, 2019

Thanks @jennybc !

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

Successfully merging a pull request may close this issue.

None yet
3 participants