Open yuanxiongbear opened 3 years ago
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
The package includes all the following forms of documentation:
setup.py
file or elsewhere.Readme requirements The package meets the readme requirements below:
The README should include, from top to bottom:
Reviewers are encouraged to submit suggestions (or pull requests) that will improve the usability of the package as a whole. Package structure should follow general community best-practices. In general please consider:
Note: Be sure to check this carefully, as JOSS's submission requirements and scope differ from pyOpenSci's in terms of what types of packages are accepted.
The package contains a paper.md
matching JOSS's requirements with:
Estimated hours spent reviewing: 2h
Hi team 👋
The pytweet
package looks great and I am impressed by the work you have to done to build this package in such a short period of time. I enjoyed testing your package using my Twitter handle and thanks to this package I've got some interesting insights about my tweets.
Below are some comments that I think would make your package even greater and easier to use.
General comments:
Overall I enjoyed using/testing your pytweet
package and I liked your rationale about the usefulness of this package and its place in the Python ecosystem as it seems like existing Python packages that perform tweets text analysis and sentiment analysis do not provide hashtag and/or timeline visualization.
Good job and keep up the good work!!
Functionality:
pip install -i https://test.pypi.org/simple/ pytweet --extra-index-url https://pypi.org/simple/
so I think that you might want to update the installation command in your installation instructions. .bash_profile
.pytweet.get_tweets('@BrunoMars', n_tweets=8)
after I imported your package based on your instructions. I think that you need to update the import of your package into something like this: from pytweet import pytweet
. I tested the get_tweets()
function after importing your package properly and it worked fine. tweet_sentiment_analysis(tweet_data)
in your instruction as it throws errors. I was able to use tweet_sentiment_analysis
like this pytweet.tweet_sentiment_analysis(tweet_data)
.plot_timeline()
function I faced this error: TypeError: strptime() argument 1 must be str, not Timestamp
. It would be great if you can handle this error gracefully by printing a more informative message about the type of argument the function is expecting.visualize_sentiment()
function I faced this error: KeyError: 'negative'
. Also, the proper way to use this function is pytweet.visualize_sentiment()
so you might want to update that line in your instructions in the README.Documentation:
pyproject.toml
file. Right now I can see only "mmyz <yzmarco.ma@gmail.com>"
.plot_type
in the visualize_sentiment
function, or at least include the plot_type
argument in the example where you call this function, e.g. pytweet.visualize_sentiment(sentiment_df, plot_type="Standard")
. Seems like Standard
is the default one, and I realized this only after I looked at the code of this function, and as it appears there are two more options, i.e. Stacked
and Separate
.get_tweets()
function, e.g. if the users want to include the replies then they can specify include_replies=True
. This argument, include_replies
, is an optional argument and defaults to False
and I saw this only after I looked at the code of this function, so it would be more obvious if you called this function by including all the arguments, e.g. pytweet.get_tweets('@BrunoMars', n_tweets=8, include_replies=False, verbose=True)
.I would like to hear your thoughts on this and let me know if you need more clarification.
Cheers, Fatime
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
The package includes all the following forms of documentation:
setup.py
file or elsewhere.Readme requirements The package meets the readme requirements below:
The README should include, from top to bottom:
Reviewers are encouraged to submit suggestions (or pull requests) that will improve the usability of the package as a whole. Package structure should follow general community best-practices. In general please consider:
Note: Be sure to check this carefully, as JOSS's submission requirements and scope differ from pyOpenSci's in terms of what types of packages are accepted.
The package contains a paper.md
matching JOSS's requirements with:
Estimated hours spent reviewing:
2 hours
Overall I think the project team has done a good job with this pytweet
package. There are some minor issues with installation to be be addressed. One of the tests failed, so it might need some checking. Overall, this is a useful package. I especially like all the functions that produce interesting plots as they are very straightforward and informative. Thanks for making this fun package!
Issue 1: Following the installation instruction from the README file, I wasn't able to successfully install the package into my local computer. The reason might have something to do with having it in only testPyPi but not in PyPi, as some of the package dependencies of testPyPi have not been updated to the latest version and thus unavailable.
Suggestion: You could consider publishing the package in PyPi so that all the packages needed are easily accessible upon installation.
You could also try using the following code:
pip install -i https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple pytweet
Issue 2: The following line of installation command on the readthedocs webpage needs to be updated to the same as the one on README file.
Issue 3: Change the following line of code:
import pytweet
to from pytweet import pytweet
Issue 4: When I ran pytweet.plot_timeline(tweet_data, 'time')
, I encountered a TypeError, which can be fixed after commenting out line 126 on pytweet.py
.
Issue 5: For the following two functions, make sure the prefix pytweet.
is added before function names.
Issue 6: When I ran the tests, one of the failed, so you might want to take a look at that.
Hope you will find the above suggestions helpful!
Cheers, Elina
Submitting Author: Huanhuan Li (@huan-ds), Yuanzhe(Marco) Ma (@mmyz88), Jared Splinter (@JaredSplinter), Yuan Xiong (@yuanxiongbear) Package Name: pytweet One-Line Description of Package: A python package that gives a brief summary on a user's tweet. Repository Link: https://github.com/UBC-MDS/pytweet Version submitted: 0.1.8 Editor: Tiffany Timbers (@ttimbers) Reviewer 1: TBD
Reviewer 2: TBD
Archive: TBD
Version accepted: TBD
Description
Scope
* Please fill out a pre-submission inquiry before submitting a data visualization package. For more info, see notes on categories of our guidebook.
Explain how the and why the package falls under these categories (briefly, 1-2 sentences):
Who is the target audience and what are scientific applications of this package ?
Are there other Python packages that accomplish the same thing? If so, how does yours differ?
If you made a pre-submission enquiry, please paste the link to the corresponding issue, forum post, or other discussion, or
@tag
the editor you contacted:Technical checks
For details about the pyOpenSci packaging requirements, see our packaging guide. Confirm each of the following by checking the box. This package:
Publication options
JOSS Checks
- [ ] The package has an **obvious research application** according to JOSS's definition in their [submission requirements][JossSubmissionRequirements]. Be aware that completing the pyOpenSci review process **does not** guarantee acceptance to JOSS. Be sure to read their submission requirements (linked above) if you are interested in submitting to JOSS. - [ ] The package is not a "minor utility" as defined by JOSS's [submission requirements][JossSubmissionRequirements]: "Minor ‘utility’ packages, including ‘thin’ API clients, are not acceptable." pyOpenSci welcomes these packages under "Data Retrieval", but JOSS has slightly different criteria. - [ ] The package contains a `paper.md` matching [JOSS's requirements][JossPaperRequirements] with a high-level description in the package root or in `inst/`. - [ ] The package is deposited in a long-term repository with the DOI: *Note: Do not submit your package separately to JOSS*Are you OK with Reviewers Submitting Issues and/or pull requests to your Repo Directly?
This option will allow reviewers to open smaller issues that can then be linked to PR's rather than submitting a more dense text based review. It will also allow you to demonstrate addressing the issue via PR links.
Code of conduct
P.S. *Have feedback/comments about our review process? Leave a comment here
Editor and Review Templates
Editor and review templates can be found here