Closed jnederlo closed 4 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:
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: 3
draft_pick
function is not consistent with the format of the other three functionsplayer_stats
and team_stats
functions are not consistent with the actual data frame outputsteam_stats
function, I was able to enter invalid season dates (such as "20022000") and no error was raised. As well, when entering the start_season as any string with a number in it, that isn't in YYYYYYYY format (such as "1999200"), and as long as the end_season argument was valid, no error was raised and a dataframe was outputted. When entering invalid arguments of strings with numbers as arguments for both arguments, or for end_season, an error wasn't raised and it resulted in an empty output. When entering a string with characters as an argument, the error raised is "Bad request", which is not informative enough. More time should be spent on ensuring valid inputs to this function.draft_pick
function, I was able to enter 0 as argument for round_number, when documentation says it should be in range of 1-25. As well, the documentation says year should be string, however passing a string in YYYY format results in error (which doesn't specify the variable type input should be).attendance
function, it would be great if the user had an option on what size the plot should be. As well, it may be easier to read the x and y axis if the plot was made into a horizontal bar graph, with the attendance axis ticks being 1M, 2M, 3M etc. player_stats
and team_stats
functions, it would be great to have explanations of what each column name means, or a link to resource that shows this, in the documentation. Though I assume most of the people who would use this package would be well versed in NHL statistics, some may not. For example, what does evGoals mean?draft_pick
function, it would be nice to have the option to input a list of pick numbers, so a user can get for example the top 3 draft picks each year.attendance
function, I think having the bars in each plot be different would make it easier to immediately differntiate between regular season and playoffs. This could also lead to giving the user the option to customize what color the user would want each plotOverall, you did an excellent job on the package, and I enjoyed using and reviewing it. If you have any questions regarding my review, please feel free to contact me.
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:
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:
Hi, everyone, my comments below:
I enjoyed the simplicity, minimalism of the documentation and usage of the function. I had a couple of issues when installing. Messages such as: Could not find a version that satisfies the requirement...
, but after installing the dependencies, the module was working fine.
I just have a couple of suggestions:
pypuck.player_stats
. I believe it could be helpful to inform users what is the oldest season that has data. I noticed that it is possible to input very old dates. Maybe it would good to display a warning message, whenever a user is trying to capture a range outside of available data range.pypuck.player_stats
: I noticed that when inputing atypical season dates such as: pypuck.team_stats(start_season='18891985', end_season='20102011')
, the function is still working. How is the function interpreting this input? And do you agree a validation should exist that guarantees startsean/endseason parameters are inputed as expected?pypuck.draft_pick
: Everything seems to be working fine. Whenever I enter too ancient dates or future dates I get a coherent exception. Good job!pypuck.attendance
: Everything seems to be working fine. Nice minimalistic visualization. Just as an idea about this function: it would be a nice feature for the user to have access to the data in form of a dataframe. (For the user to do the plot he pleases). What about having a parameter that makes the user decide if he wants a plot or a dataframe an output, also a parameter deciding if the data should be aggregated or shown by game?Please let me know what to you think about this.
By looking at how good and clean your function works, I can only imagine the data that could be extracted in other sports as well!
Thanks for your feedback and suggestions. We went through and addressed most of your suggestions, in-fact we addressed all of the suggestions that didn't introduce new features as those can/should be addressed in future major releases. To specifically summarize what we addressed:
@ofer-m: Addressed:
Did Not Address:
The suggestions that we did not address are good suggestions that we will put on our radar for future releases.
@manu2856: Addressed:
Did Not Address:
You can view these changes as part of release v1.1.2.
Submitting Author: Jarvis Nederlof (@jnederlo), Xugang Zhong (@chuusan), Polina Romanchenko (@PolinaRomanchenko), Manish Joshi (@ManishPJoshi ) Package Name: pypuck One-Line Description of Package: This package provides wrapper functions to access the publicly available but undocumented NHL.com API. Repository Link: pypuck Version submitted: v1.1.0 Editor: Varada Kolhatkar (@kvarada) Reviewer 1: Ofer Mansour (@ofer-m) Reviewer 2: Manuel Maldonado (@manu2856) Archive: TBD Version accepted: TBD
Description
Scope
* Please fill out a pre-submission inquiry before submitting a data visualization package. For more info, see this section of our guidebook.
@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](https://joss.readthedocs.io/en/latest/submitting.html#submission-requirements). 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](https://joss.readthedocs.io/en/latest/submitting.html#submission-requirements): "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](https://joss.readthedocs.io/en/latest/submitting.html#what-should-my-paper-contain) 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