-
-
Notifications
You must be signed in to change notification settings - Fork 104
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
Submission: rtweet #302
Comments
As I said in openjournals/joss-reviews#1454 -- I'll happily review the package here. I've reviewed for JOSS (alongside @andrewheiss, in fact), and would gladly review for @ropensci :-) |
I tentatively listed @andrewheiss and @briatte as reviewers because they initially expressed their willingness to review on a [now closed] JOSS thread. I don't want to presume they are/will be the official reviewers–I'm not completely familiar with the ropensci process, but I'd imagine the selection of reviewers isn't entirely up to me–but I figured reviewer suggestions/volunteers were worth knowing! |
thanks for the reviewer suggestions, much appreciated. |
Editor checks:
Editor commentsThanks for your submission @mkearney ! Here's the output from goodpractice. If you haven't used
── GP rtweet ─────
It is good practice to
✖ avoid long code lines, it is bad for readability. Also, many people prefer editor windows that are about 80 characters wide. Try make your lines
shorter than 80 characters
R/bearer_token.R:56:1
R/bearer_token.R:80:1
R/coords.R:119:1
R/coords.R:131:1
R/coords.R:132:1
... and 37 more lines
✖ avoid sapply(), it is not type safe. It might return a vector, or a list, depending on the input data. Consider using vapply() instead.
R/favorites.R:75:45
R/post.R:326:12
R/post.R:336:12
R/stream.R:338:13
R/utils.R:327:40
... and 1 more lines
✖ avoid the library() and require() functions, they change the global search path. If you need to use other packages, import them. If you need to
load them explicitly, then consider loadNamespace() instead, or as a last resort, declare them as 'Depends' dependencies.
R/utils.R:485:5
✖ fix this R CMD check NOTE: Note: found 113868 marked UTF-8 strings
✖ checking tests ... SEE QUESTION ABOVE Seeking reviewers now 🕐 Reviewers:
|
Here's Package Review (version 2, final)Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide
DocumentationThe package includes all the following forms of documentation:
Functionality
Final approval (post-review)
Estimated hours spent reviewing: 0.5 (draft version 1)
Review CommentsReviewed using R version 3.6.0 on x86_64-apple-darwin15.6.0 (64-bit). Installed with devtools::install_github("mkearney/rtweet", dependencies = TRUE) Tested with both the "out of the box" access to the Twitter API and personal OAuth credentials via Typo at https://rtweet.info/articles/auth.html -- "autheticate via web browser". I have only one request beyond what Andrew is suggesting in terms of better documenting how to fix the tests (excellent table suggestion, by the way, Andrew!): Could the thread about how I have used both Last, something not related to the package itself: since Twitter has sunset |
Package ReviewPlease check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide
DocumentationThe package includes all the following forms of documentation:
Functionality
Final approval (post-review)
Estimated hours spent reviewing: ≈5 hours
Review CommentsDocumentation and vignettesAll vignettes worked great and were a perfect introduction to the package. It might be helpful to move up the vignette listing near the beginning of the README so that users can find them more easily. Each function is generally very well documented. There are some sparsely documented functions like It's really helpful that the US and the world are baked into The pkgdown site is great. It might be helpful (and recommended by rOpenSci) to add Installation and testingInstallation worked fine with Testing doesn't work out of the box and it took me a while to figure out how to generate the
It might be useful to explain that process somewhere in the documentation—not in the README though, since it's not something a typical user would do. 190 tests passed (super impressive coverage!!), and 5 tests failed:
Token creation and authenticationIt's so great that people can use this without creating their own Twitter apps, since that process has become long and arduous. It might be helpful to more clearly explain the benefits of creating an app with tokens vs. just using the interactive browser-based token. Right now, the README says "You may still choose to do this [create an app] (gives you more stability and permissions)", and the auth vignette explains how to do it, but there isn't any explanation of why users might want to do it. Later in the README, under "Post actions," it explains that users would need their own app to post tweets and access DMs, but it could be helpful to have that information up above when describing authentication in general. It might even be helpful to have a table of sorts showing pros/cons/features available when using a private app vs. the embedded
The page for creating apps looks slightly different from the screenshots, and the direction to go to apps.twitter.com is out of date—according to Twitter, that aspect of developer accounts is getting sunset. Instead, the URL is now https://developer.twitter.com/en/apps . CommunityIt would be helpful to have some community guidelines in the package too, such as a CONTRIBUTING.md file (perhaps modeled after something like this) and a CONDUCT.md file (like this) Creating an RStudio project rOpenSci-related issues
JOSS-related issuesThere are a few issues with
Kicking the tires with some use casesLook at tweets
Geocoding with sf library(sf)
library(rnaturalearth)
searched_tweets <- search_tweets(
q = "lang:en",
geocode = lookup_coords("usa"), n = 10000
) %>%
lat_lng() %>%
drop_na(lng) %>%
sf::st_as_sf(coords = c("lng", "lat"), crs = 4326)
us <- ne_states(country = "united states of america", returnclass = "sf") %>%
filter(!(postal %in% c("HI", "AK", "PR")))
ggplot() +
geom_sf(data = us) +
geom_sf(data = searched_tweets) +
coord_sf(crs = 102003, datum = NA) + # Albers
theme_void() Magical. This is a great package—fantastic work! Tested and reviewed using R 3.6.0 on x86_64-apple-darwin15.6.0 (64-bit). |
thanks for your review @andrewheiss - @briatte let me know when you're done reviewing |
Hi @sckott I have updated my draft review, which can be considered final. My new comments are at the bottom of the review. In a nutshell, I hit the same issues as @andrewheiss did with the tests, and am asking for additional details to be included about how the package compares to Generally speaking, the package is awesome, thanks a lot for your work @mkearney! |
FYI, I've just posted an issue to Perhaps this issue could lead to better documentation re: how to collect large amounts of followers/friends? As the issue says, I am unsure that I have understood how Last and still related to followers, I subscribe to the suggestion made here to harmonize the output of |
First, thanks to @briatte and @andrewheiss for taking the time to review rtweet and providing such great feedback! Second, please see my detailed reply to each reviewer below. This revision process has so far resulted in a lot of changes to rtweet–many of which were quite difficult😅–but I am quite certain the package is much better for it. Thank, -Mike @briatte
@andrewwheissDocumentation and vignettes
Installation and testing
Token creation and authentication
Community
JOSS-related issues
|
thx for your responses to reviews @mkearney any feedback/thoughts on the comments reviewers ? (@briatte @andrewheiss ) |
Other than a duplicate "Premium 30 day" row in the big comparison table, this addresses all my concerns and I think it's good to go ✅ |
@mkearney since you're keen on contributing a blog post once the review process is complete 🙂 ... Here are some technical and editorial guidelines: https://github.com/ropensci/roweb2#contributing-a-blog-post. Publication date is flexible. I like to get a draft via pull request a week before the planned publication date so I can review. Happy to answer any questions. I'm thrilled that you've put rtweet through review. |
@briatte Are you happy with the changes made? |
Any update on this? |
@mkearney let's move on. I'll take a final look and get back to you ASAP |
looks good, just a single comment: Approved! Thanks @mkearney for submitting and @briatte @andrewheiss for your reviews! To-dos:
For JOSS:
We've put together an online book with our best practice and tips, this chapter starts the 3d section that's about guidance for after onboarding. Please tell us what could be improved, the corresponding repo is here. |
@sckott I’m not currently seeing an invite (and I wasn’t authorized to transfer to ropensci). Where would I find this? |
@mkearney sorry, now you should have gotten an invite ... let me know if you don't see it soon |
Got it! Thanks! |
TO DO for rOpenSci:
TODO for JOSS:
We've put together an online book with our best practice and tips, this chapter starts the 3d section that's about guidance for after onboarding. Please tell us what could be improved, the corresponding repo is here. |
@mkearney When would you like to publish the post? Tues Nov 12 is earliest date available, and that would mean submitting a draft via pull request by Tues Nov 5. Later dates are also open. Details #302 (comment) |
Docs are now live on https://docs.ropensci.org/rtweet/ |
@stefaniebutland Nov 5th/12th works for me! |
@mkearney Will you submit a draft post soon? |
Submitting Author: Michael W. Kearney (@mkearney)
Repository: https://github.com/mkearney/rtweet
Version submitted: v0.6.9
Editor: @sckott
Reviewer 1: @andrewheiss
Reviewer 2: @briatte
Archive: TBD
Version accepted: TBD
Scope
Please indicate which category or categories from our package fit policies this package falls under: (Please check an appropriate box below. If you are unsure, we suggest you make a pre-submission inquiry.):
Explain how and why the package falls under these categories (briefly, 1-2 sentences):
Data retrieval because the package allows users to easily request and import data from Twitter's REST and stream APIs.
Data munging because significant work is done to convert JSON objects returned by Twitter's APIs into tabular data frames.
The target audience is researchers. The scientific applications span a range of topics. Here's is the relevant excerpt from paper.md
To date no other package interfaces with both REST and stream APIs. The twitteR package is most similar, but it has entered a stage of deprecation (I've agreed to carry the torch, so to speak). So, not only does twitteR not reflect some recent changes to Twitter's API (most notably the introduction of 'extended tweet' mod–the new 280 character limit), but it lacks active maintenance thanks–in part–to rtweet filling the void.
#193
Technical checks
Confirm each of the following by checking the box. This package:
Publication options
JOSS Options
paper.md
matching JOSS's requirements with a high-level description in the package root or ininst/
.MEE Options
Code of conduct
The text was updated successfully, but these errors were encountered: