-
-
Notifications
You must be signed in to change notification settings - Fork 39
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
[REVIEW]: Handprint: a program to explore and compare major cloud-based services for handwritten text recognition #4328
Comments
Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks. For a list of things I can do to help you, just type:
For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:
|
|
Wordcount for |
|
@step21 and @rlskoeser - Thanks for agreeing to review this submission. As you can see above, you each should use the command As you go over the submission, please check any items that you feel have been satisfied. There are also links to the JOSS reviewer guidelines. The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention We aim for reviews to be completed within about 2-4 weeks. Please let me know if either of you require some more time. We can also use Whedon (our bot) to set automatic reminders if you know you'll be away for a known period of time. Please feel free to ping me (@danielskatz) if you have any questions/concerns. |
Review checklist for @step21Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
Review checklist for @rlskoeserConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
👋 @step21 & @rlskoeser - A couple of weeks in, I just wanted to check on how the reviews are coming. |
@danielskatz thanks for the nudge. Last week was busy, but I should be able to spend some time on it this week. |
@danielskatz question about the license: it's described in the readme as a "a BSD/MIT type license" but I don't think it's actually any of the licenses on the OSI list. Here's the full description in the readme:
Please advise. |
It's almost https://opensource.org/licenses/BSD-3-Clause but not quite, and not quite isn't good enough, as it's not an OSI-approved license then. @mhucka, can you remove line 2 ("All rights not granted herein are expressly reserved by Caltech.") from the license files? And change the readme to say that this uses the 3-Clause BSD License, rather than "a BSD/MIT type license"? |
Well, hmmm, this was handed down to us by the institute's IP devision, but nevertheless, I'm asking if this policy can be revisited. I'll apprise people here of the answer once I know something. |
Ok - @mhucka, just so that it's clear, we can't publish this work without an OSI-approved license |
Yup, understood. |
@mhucka - any news on this license issue? |
Not yet. I just pinged them again ... |
HI @mhucka - I'm just checking on this again... |
@danielskatz At precisely 5:01 pm today, I finally received an email in answer to the license question. The answer is, yes, for this project (and only this project :-)) we can change the license to be straight BSD. I will update the license file and any associated files and issue a new release of the software. It will not take long – hopefully tomorrow or Thursday at the latest. |
👋 @step21 & @rlskoeser, sorry for the delay - we might have caught the license issue earlier. But in any case, we should now proceed with (continue) the review of the software. I look forward to seeing any more issues you find, or seeing the lack of them. 🙂 |
👋 @step21 & @rlskoeser - please let me know if there's anything blocking you from continuing your reviews |
Hey, there is probably nothing blocking me, but I have been sick all week. This definitely does not increase my review capability as I am now behind on everything. I hope to be able to work again next week but in all likelihood, at least for a thorough review I can only start the week after. |
@step21 - thanks for the update |
I've released version 1.6.0 of the software, with a proper unmodified BSD 3-clause license. |
@rlskoeser Thanks for spotting that. I guess the html version is not really needed anyway, so I'll delete it right now. Thanks again! |
As [pointed out by @rlskoeser](openjournals/joss-reviews#4328 (comment)), the LICENSE.html file confuses GitHub's license navigator. I don't think I use it for anything anymore, so let's just delete it.
@mhucka great. Checking off the license now on my checklist. 🙂 |
I had some trouble getting testing access at least with google. Azure worked better but did not test yet. Haven't tried Amazon yet. If you would have any test credentials, this could help as I think I might have to sign up for a whole new Google account to get testing credit again. |
Issue re paper that there is no comparison to the field and maybe too much much usage description in the paper. caltechlibrary/handprint#38 |
Also I would prefer if a direct usage example would also be present in the wiki, not only in the documentation site. caltechlibrary/handprint#39 |
I agree with @step21 about the paper. Added some comments on caltechlibrary/handprint#38 to elaborate on what could be condensed/simplified and what I'm interested to learn more about that isn't currently included. |
@rlskoeser did you do any functionality testing yet? My main problem with that is that I did not yet manage to get a google account with free credit - only for azure, and amazon I haven't tried yet. Mostly I would probably test with images from the tests in the software repository, though these are limited as they these are just a few examples of course. |
@step21 I tested against google because I already had a project setup that I was able to create credentials for. I don't have projects and accounts with the others and was holding off setting them up until I have a block of time that I'm confident I'll be able to get everything set up and tested. I tested with an image from a project I'm currently working on, Princeton Geniza Project because I'm genuinely interested to know how well these services can do with this content (handwritten medieval content in Hebrew and Arabic script and a variety of languages). |
Code documentation doesn't include a statement of need; the statement of need in the JOSS paper should clarify the target audience. |
@rlskoeser ok, thanks for the info, that is helpful. |
Not sure whether or not to check off the automated tests — there are some python tests, but it's a bit hard to tell which parts of the code they cover. There is no CI set up as far as I can tell, and no documentation on how to run the tests, although I was able to run them fairly easily with pytest. (It would probably be straightforward to set up GitHub Actions to install the app and run pytest; also easy to document running them, maybe in the contributing doc.) I looked at the reviewer guidelines and it doesn't seem like this quite fits in any of those categories — not automated CI, not documented; don't see any instructions on how to test expected functionality manually with sample input. Also not sure how to judge if the tests cover the "core functionality" of the application. @danielskatz what do you advise? |
There is thorough documentation for installing, configuring, and using handprint from the command line. However, I'm not finding any code-level documentation. @mhucka in the JOSS paper under the statement of need, you say that handprint could be used in "scripts as part of automated workflows." Is it your expectation that anyone using it that way would call handprint from the command line? If I'm working in python, could I use functionality from handprint as a code library without resorting to a command line call? Does handprint expose any kind of reliable API that would be safe/appropriate/reliable to use this way, or is the code (or part of it) internal and shouldn't be used that way? My sense of your options is that you can either:
Maybe @step21 or @danielskatz will have thoughts on other options here. I'm probably thinking about it this way because as a python developer, I'd want to call it from my code for anything automated / bulk operation; but maybe I'm not your target audience! :-) If you don't want to expose a python API, it would be nice to add links to other existing tools/libraries to use instead for that kind of work. I'm trying to understand what happens after someone uses handprint to evaluate options: once they determine the best solution, can they keep using handprint or do they need to / should they switch to a different tool? |
@rlskoeser I also think it looks more like it is meant to be automated via shellscript (or Python calling a shellscript) I tested it with Azure for now on the IAM Handwriting Database. (https://fki.tic.heia-fr.ch/databases/iam-handwriting-database) - the functionality works as far asI can tell. (with sporadic checks of results) It is not especially fast, but as far as I can see there are also no claims to that end, and it might depend on the service and account. This could probably be sped with parallel processing, if the service allows it.
Especially the Amazon limitation seems kind of low to me. |
👋 @mhucka - any response on the 3 comments above from @rlskoeser and @step21? |
It looks like a bit more is needed from @mhucka. As you say, JOSS does not require CI and automated tests, but the manual tests do need to be clear to understand what they do and what is being tested. |
👋 @mhucka - we need your input/actions here!! |
Regarding @step21's comment about "Especially the Amazon limitation seems kind of low to me": that's from Amazon's service – there is nothing I can do about that. I tried to make it clear that it's the API that returns those results. Regarding @step21's comment about "This could probably be sped with parallel processing": actually, Handprint already uses parallel processing. It sends the input images to separate services in parallel. There is not much that can be done about parallelizing the processing of the output of a given service for a given page, unfortunately. |
I appreciate people's efforts and comments. Regarding comparison to the field: doing a proper exploration of the state of the field today would take hours of work, which unfortunately I don't have. The last time I looked, there were no comparable tools, which is why there is no comparison in the paper currently. If the need for a review of the state of the field is necessary, then I'm afraid I'm going to have to shelve this, because I regret I simply don't have the time. |
@mhucka - one of the JOSS review criteria is "State of the field: Do the authors describe how this software compares to other commonly-used packages?" If you think there are no other commonly-used packages that are comparable, you might just say so in the paper. |
@mhucka - we do need to keep making progress on this |
I am sorry, but I lack the time to do more right now. Realistically, the options for me right now are: (1) pause for a couple of months or (2) withdraw the submission. |
👋 @mhucka - I think we should mark this as withdrawn, which I will do. If you decide to resubmit, please mention this issue (#4328) when you do so, and we can see if the same reviewers are available. 👋 @step21 & @rlskoeser, I'm sorry that your effort will not lead to a JOSS publication at this point, but thanks very much for your work in any case. |
@editorialbot withdraw |
Paper withdrawn. |
Thanks @danielskatz for your work on it too. @mhucka sorry it didn't work out at this point. |
@rlskoeser @step21 Thank you, very much, for your time and efforts on the review. |
No worries, it was a pleasure. A good rest of the week to everyone! |
Submitting author: @mhucka (Michael Hucka)
Repository: https://github.com/caltechlibrary/handprint
Branch with paper.md (empty if default branch): joss-paper
Version: 1.5.6
Editor: @danielskatz
Reviewers: @step21, @rlskoeser
Archive: Pending
Status
Status badge code:
Reviewers and authors:
Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)
Reviewer instructions & questions
@step21 & @rlskoeser, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review.
First of all you need to run this command in a separate comment to create the checklist:
The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @danielskatz know.
✨ Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest ✨
Checklists
📝 Checklist for @step21
📝 Checklist for @rlskoeser
The text was updated successfully, but these errors were encountered: