Open zjr-mds opened 2 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:
I'm not sure why the source files have each function separately and then combined together in animalgonewild.py
. If they are identical, you can import them from the respective files to avoid editing mistakes in the future.
TextTransformer
only works for lower case proper nouns, it should convert to lowercase before the process.
AnimalType
output a PIL file that is unable to view when running in the terminal, suggest using .show() for the output.
worldCloud
the output also cannot be viewed using the terminal.
worldCloud
function take an url
, but the readme stated a str
. I guess technically they are equivalent, but you should fix it to avoid any confusion.
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: 1.5hr
Great work overall. Here are some of my comments regarding this package:
I tested the testTransformer
function and found it returns differently from the help function example.
It looks like the function replaced the adjective
silly
into an animal too.
As Son mentioned, the outputs for worldCloud
and also for animalType
return a image, and from testing in both terminal and PyCharm, I got a line representing the image instead of the actual image. For example, here is the output that I got: <PIL.PngImagePlugin.PngImageFile image mode=RGBA size=300x250 at 0x10BA0E130>
. Maybe you can provide a note or some alternatives for checking the actual images from these two functions from terminal or other tools.
In the README file, consider providing some brief coding examples on how to use your functions. Also, minor but it might be a good idea to format the function names into coding format.
Minor: Lack of indentation for the line Returns...
inside the docstring in textTransformer
In README file, in addition to the pip install
instruction, also consider include how to import the package and the functions to make it easier for readers to start.
@lynnwbl Thank you for the helpful feedback that we could be able to fix some of the minor issues before milestone4 submission! However, for the textTransformer, the database we're using can't distinguish the word with perfect accuracy if it could be both noun and adj, like 'silly' which can be used as both a noun or an adjective. So far, any words like this would always be treated as nouns.
@SonQBChau Thank you for the great tips! The reason we had separate files was that we were initially working separately but we had a reviewer compile the functions into one single file before the submission, but maybe we should change the name to avoid confusion. Thanks for catching the issue with the lower case nouns!
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:
Great job, group 16, I like the interesting package and I enjoy the time that I play around with different strings.
Here's something could make the package even better:
animalType
function, the graph did not render properly. I think you guys may need to add some instructions on how to make the function works as people would like to see those interesting plots.wordCloud
function, the picture did not render in the terminal as well. It would be great if I can see those pictures when I run the function!animalClassifier
can justify the reason why the certain animal was returned. Also, if it is possible, you guys could add more animals to the list!textTransformer
function did not work properly with verbs as well. It will transform a verb into an animal as well. When I typed "I wanna go", it returned "I wanna Whale". Maybe the group can further edit on this function to avoid the issue!
animalType
function does not have codes, just docstring. I think it could be a good practice to save codes consistently as other functions do have codes in the src folder. Other than that, great works overall. Keep developing on it!
Best, Zack
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: 1 hr
This package is really entertaining and very nicely documented with clear instructions. Most of the points have already been addressed and therefore, have just few suggestions to take it further:
Overall, really great work group 16, very structured, humorous and entertaining way of doing things.
Submitting Author Name:
Package Name: Ranimalsgonewild One-Line Description of Package: Python package for text analysis with an animal theme Repository Link: https://github.com/UBC-MDS/animalsgonewild Version submitted: 1.1.1 Reviewer 1: Shiva Jena (@shiva) Reviewer 2: Lynn Wu (@lynnwbl) Reviewer 3: Son Chau (@SonQBChau) Reviewer 4: Zack Tang (@zackt113)
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