-
Notifications
You must be signed in to change notification settings - Fork 35
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
Rebuttal letter #248
Rebuttal letter #248
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drafting a few more responses in the review comments:
A clear distinction between CPython and Python is stressed, which might not be relevant to many audiences as alternatives like PyPI are likely not well understood. This is also an example of where the paper may delve too much into technical details.
We've removed the sentence referring to CPython on page 2 of the manuscript--we agree that this is indeed too technical to be of interest for the likely audience.
Page 3. Figure 1. There should be a date on when the announcement was initially released and potentially medium used.
The exact date of the announcement and the name of the mailing list that was used have been added to the caption to Figure 1 on page 3 of the manuscript.
Page 5. A very brief mention of what GitHub is should be stated; bench chemist may be unaware of this platform.
We've briefly described GitHub in a new sentence added to page 5 of the manuscript, in the "SciPy Today" section.
Page 5. Figure 2. The term “SciKit” is only briefly mentioned before this figure and is never fleshed out. With the current phrasing, it is unclear if SciKits are competitors or extensions of SciPy, their relationship should be well defined.
The nature of scikits has now been discussed in more detail in a modified sentence on page 4 of the manuscript, prior to their mention in Figure 2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More rebuttal entry drafts, again focused on reviewer 2:
Page 7. Many Fortran packages like ODEPACK and ARPACK are discussed without context. Readers are unlikely to be familiar with these libraries (even BLAS is questionable). While enumerating the capabilities of each package is clearly beyond scope simply categorizing them in some fashion would be worthwhile.
Page 7 of the revised manuscript has now been revised to include categorization of the external Fortran packages into 6 distinct categories, based on what the libraries provide. This is in the "Language choices" section.
Page 11. From the text, it is unclear why testing and code coverage are useful metrics. Some mention of correctness is likely required and could be a good time to mention that these metrics give both contributors courage that they are not breaking the package and maintainers some faith that a contribution does not harm the package.
Page 11 of the revised manuscript now contains a new paragraph briefly outlining the value & purpose of test-driven development, including the suggested mention of increasing confidence in code changes.
Page 11. The compiled coverage of 45% seems surprisingly low and perhaps should be remarked on. The maintainers also appear to have allowed remarkable amounts of untested compiled code in recent years. One could make an argument that the correctness of SciPy is much more important than the feature set as it is currently thought of as a dependable resource.
The "test suite" subsection on page 11 has been revised to clarify that compiled code is actually more robust than the 45 % coverage figure might suggest given a variety of factors including reputable vendored code and large amounts of automatically-generated code for which full coverage is impractical.
Links to SciPy cookbooks or tutorials would be useful to highlight to give readers immediate examples of how SciPy would be helpful. These tutorials are only linked in your Discussion section and maybe missed.
We've added an additional mention of the SciPy Tutorial and API Reference much earlier in the manuscript now--in a new sentence in the "Package organization" subsection on page 6.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other minor comment from Reviewer 1 to draft a response for:
Scientific software is depressingly underfunded. I imagine the economic cost of SciPy is in the tens (if not hundreds) of millions of dollars yet, as you mention, the actual direct financial contributions were a magnitude (or two) less. How did the SciPy community address this disparity? Are there lessons to be learned for the larger scientific community?
We've added a brief new subsection named "Funding" starting on page 13 of the revised manuscript, which touches on the estimated > 10 M dollar development cost of SciPy, and what may need to improve for sustainability moving forward. We still depend largely on volunteered time.
Ok, that's just about all of the responses drafted except for the CI revision relocation issue #252. Matt may want to revise some of the drafts above & include i.e,. the GitHub discussion links for consistency with what he has already done, but we should otherwise be getting close. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking pretty good to me. I added two minor suggestions -- we could also just say the comments are addressed in the order provided in the original reviews, with the exception of the combined GPU / future comments.
I counted through the number of rebuttals & it seems to match the number of bullet points from both reviewers if you account for fusing the GPU comments & then adding one more revision for background streamlining more generally.
\begin{newlfm} | ||
|
||
We appreciate the careful review and helpful suggestions from you and the reviewers. Each comment is quoted and addressed below; we also link the relevant GitHub issues in which we discussed and implemented each suggestion. As requested, the corresponding changes are underlined in the revised manuscript. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe add something like "... except for minor changes in the references section that couldn't be highlighted due to a technical issue" as @jarrodmillman suggested in #251.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should manually highlight these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So far we're 2 to 1 in favor of just briefly mentioning the major additions to refs & saying they weren't highlighted due to a technical issue. I or you could probably come up with a list and do it manually, but let's not dwell on this for too long...:
- the new unit testing textbook citation
- the new OpenHub citation
- the new Nature News CI article citation
- any others?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from those minor suggestions, I'm optimistic that resubmission of the revised manuscript is now imminent. @rgommers any final concerns on this? |
alright, merging |
I appreciate the attention to detail in the rebuttal, and the references to PRs for each piece of work. Thanks! |
Initial draft includes only response about heterogeneous computing.
(Eventually) closes #247