Skip to content
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

[PRE REVIEW]: GrainSizeTools: a Python script for grain size analysis and paleopiezometry based on grain size #811

Closed
whedon opened this issue Jul 7, 2018 · 49 comments

Comments

@whedon
Copy link

whedon commented Jul 7, 2018

Submitting author: @marcoalopez (Marco A. Lopez-Sanchez)
Repository: https://github.com/marcoalopez/GrainSizeTools/
Version: v2.0
Editor: @lheagy
Reviewers: @jsta

Author instructions

Thanks for submitting your paper to JOSS @marcoalopez. The JOSS editor (shown at the top of this issue) will work with you on this issue to find a reviewer for your submission before creating the main review issue.

@marcoalopez if you have any suggestions for potential reviewers then please mention them here in this thread. In addition, this list of people have already agreed to review for JOSS and may be suitable for this submission.

Editor instructions

The JOSS submission bot @whedon is here to help you find and assign reviewers and start the main review. To find out what @whedon can do for you type:

@whedon commands
@whedon
Copy link
Author

whedon commented Jul 7, 2018

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks. @lheagy it looks like you're currently assigned as the editor for this paper 🎉

For a list of things I can do to help you, just type:

@whedon commands

@whedon
Copy link
Author

whedon commented Jul 7, 2018

Attempting PDF compilation. Reticulating splines etc...

@whedon
Copy link
Author

whedon commented Jul 7, 2018

PDF failed to compile for issue #811 with the following error:

/app/vendor/ruby-2.3.4/lib/ruby/2.3.0/find.rb:43:in block in find': No such file or directory (Errno::ENOENT) from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/find.rb:43:in collect!'
from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/find.rb:43:in find' from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-e0f72c5e8125/lib/whedon/processor.rb:57:in find_paper_paths'
from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-e0f72c5e8125/bin/whedon:32:in prepare' from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor/command.rb:27:in run'
from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor/invocation.rb:126:in invoke_command' from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor.rb:387:in dispatch'
from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor/base.rb:466:in start' from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-e0f72c5e8125/bin/whedon:99:in <top (required)>'
from /app/vendor/bundle/ruby/2.3.0/bin/whedon:22:in load' from /app/vendor/bundle/ruby/2.3.0/bin/whedon:22:in

'

@arfon
Copy link
Member

arfon commented Jul 7, 2018

👋 @lheagy - the submitting author suggested you as the handling editor.

@arfon
Copy link
Member

arfon commented Jul 7, 2018

@whedon generate pdf

@whedon
Copy link
Author

whedon commented Jul 7, 2018

Attempting PDF compilation. Reticulating splines etc...

@whedon
Copy link
Author

whedon commented Jul 7, 2018

PDF failed to compile for issue #811 with the following error:

/app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:379:in parse': (tmp/811/paper.md): mapping values are not allowed in this context at line 2 column 22 (Psych::SyntaxError) from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:379:in parse_stream'
from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:327:in parse' from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:254:in load'
from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:475:in block in load_file' from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:474:in open'
from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:474:in load_file' from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-e0f72c5e8125/lib/whedon.rb:68:in initialize'
from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-e0f72c5e8125/lib/whedon/processor.rb:32:in new' from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-e0f72c5e8125/lib/whedon/processor.rb:32:in set_paper'
from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-e0f72c5e8125/bin/whedon:37:in prepare' from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor/command.rb:27:in run'
from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor/invocation.rb:126:in invoke_command' from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor.rb:387:in dispatch'
from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor/base.rb:466:in start' from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-e0f72c5e8125/bin/whedon:99:in <top (required)>'
from /app/vendor/bundle/ruby/2.3.0/bin/whedon:22:in load' from /app/vendor/bundle/ruby/2.3.0/bin/whedon:22:in

'

@arfon
Copy link
Member

arfon commented Jul 7, 2018

👋 @marcoalopez - this PR should fix the paper compilation issue: marcoalopez/GrainSizeTools#1

@marcoalopez
Copy link

@whedon generate pdf

@whedon
Copy link
Author

whedon commented Jul 10, 2018

Attempting PDF compilation. Reticulating splines etc...

@whedon
Copy link
Author

whedon commented Jul 10, 2018

PDF failed to compile for issue #811 with the following error:

/app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:379:in parse': (tmp/811/paper.md): mapping values are not allowed in this context at line 2 column 22 (Psych::SyntaxError) from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:379:in parse_stream'
from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:327:in parse' from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:254:in load'
from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:475:in block in load_file' from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:474:in open'
from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:474:in load_file' from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-e0f72c5e8125/lib/whedon.rb:68:in initialize'
from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-e0f72c5e8125/lib/whedon/processor.rb:32:in new' from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-e0f72c5e8125/lib/whedon/processor.rb:32:in set_paper'
from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-e0f72c5e8125/bin/whedon:37:in prepare' from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor/command.rb:27:in run'
from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor/invocation.rb:126:in invoke_command' from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor.rb:387:in dispatch'
from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor/base.rb:466:in start' from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-e0f72c5e8125/bin/whedon:99:in <top (required)>'
from /app/vendor/bundle/ruby/2.3.0/bin/whedon:22:in load' from /app/vendor/bundle/ruby/2.3.0/bin/whedon:22:in

'

@arfon
Copy link
Member

arfon commented Jul 10, 2018

@marcoalopez - I think you need to quote your title, i.e. change:

THIS
title: GrainSizeTools: a Python script for grain size analysis and paleopiezometry based on grain size

TO:
title: "GrainSizeTools: a Python script for grain size analysis and paleopiezometry based on grain size"

@marcoalopez
Copy link

@arfon I already fixed the whitespace and the title issues in my repository. I'm not sure how to proceed now or if this is the right thing to do.

@lheagy
Copy link
Member

lheagy commented Jul 10, 2018

Thanks @marcoalopez we can try rebuilding the pdf to make sure it is now working.

@lheagy
Copy link
Member

lheagy commented Jul 10, 2018

@whedon generate pdf

@whedon
Copy link
Author

whedon commented Jul 10, 2018

Attempting PDF compilation. Reticulating splines etc...

@whedon
Copy link
Author

whedon commented Jul 10, 2018

PDF failed to compile for issue #811 with the following error:

/app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:379:in parse': (tmp/811/paper.md): could not find expected ':' while scanning a simple key at line 18 column 1 (Psych::SyntaxError) from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:379:in parse_stream'
from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:327:in parse' from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:254:in load'
from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:475:in block in load_file' from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:474:in open'
from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:474:in load_file' from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-e0f72c5e8125/lib/whedon.rb:68:in initialize'
from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-e0f72c5e8125/lib/whedon/processor.rb:32:in new' from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-e0f72c5e8125/lib/whedon/processor.rb:32:in set_paper'
from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-e0f72c5e8125/bin/whedon:37:in prepare' from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor/command.rb:27:in run'
from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor/invocation.rb:126:in invoke_command' from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor.rb:387:in dispatch'
from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor/base.rb:466:in start' from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-e0f72c5e8125/bin/whedon:99:in <top (required)>'
from /app/vendor/bundle/ruby/2.3.0/bin/whedon:22:in load' from /app/vendor/bundle/ruby/2.3.0/bin/whedon:22:in

'

@arfon
Copy link
Member

arfon commented Jul 10, 2018

Looks like you need to change:

affiliations:
  - name: Present address - Géosciences Montpellier, Université de Montpellier & CNRS, CC 60, Place E.
7 Bataillon, 34095 Montpellier cedex 5, France
    index: 1
  - name: Departamento de Geología, Universidad de Oviedo, c/Jesús Arias de Velasco s/n, 33005, Oviedo, Spain
    index: 2

to:

affiliations:
  - name: Present address - Géosciences Montpellier, Université de Montpellier & CNRS, CC 60, Place E. 7 Bataillon, 34095 Montpellier cedex 5, France
    index: 1
  - name: Departamento de Geología, Universidad de Oviedo, c/Jesús Arias de Velasco s/n, 33005, Oviedo, Spain
    index: 2

@marcoalopez
Copy link

@arfon @lheagy Ok I've already fixed it. It seems that the software I use to edit the markdown document cut that long line. I'm sorry for all the inconvenience I've created. I hope it's fixed! Thank you very much for your help.

@arfon
Copy link
Member

arfon commented Jul 10, 2018

@whedon generate pdf

@whedon
Copy link
Author

whedon commented Jul 10, 2018

Attempting PDF compilation. Reticulating splines etc...

@whedon
Copy link
Author

whedon commented Jul 10, 2018

PDF failed to compile for issue #811 with the following error:

% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed

0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0
100 12 0 12 0 0 153 0 --:--:-- --:--:-- --:--:-- 155
Error reading bibliography ./paper.bib (line 43, column 5):
unexpected "u"
expecting space, ",", white space or "}"
Error running filter pandoc-citeproc:
Filter returned error status 1
Looks like we failed to compile the PDF

@arfon
Copy link
Member

arfon commented Jul 10, 2018

@marcoalopez - looks like you're missing a comma here: https://github.com/marcoalopez/GrainSizeTools/blob/master/paper.bib#L42

@marcoalopez
Copy link

@arfon fixed the missing comma. Again, thanks for your patience.

@arfon
Copy link
Member

arfon commented Jul 10, 2018

@whedon generate pdf

@whedon
Copy link
Author

whedon commented Jul 10, 2018

Attempting PDF compilation. Reticulating splines etc...

@whedon
Copy link
Author

whedon commented Jul 10, 2018

PDF failed to compile for issue #811 with the following error:

% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed

0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0
100 12 0 12 0 0 155 0 --:--:-- --:--:-- --:--:-- 157
Error reading bibliography ./paper.bib (line 46, column 24):
unexpected "a"
expecting space or ","
Error running filter pandoc-citeproc:
Filter returned error status 1
Looks like we failed to compile the PDF

@arfon
Copy link
Member

arfon commented Jul 10, 2018

@marcoalopez - looks like Pandoc can't handle bibtex keys with spaces in them (e.g. Lopez-Sanchez and Llana-Funez:2015).

@marcoalopez
Copy link

@whedon generate pdf

@whedon
Copy link
Author

whedon commented Jul 10, 2018

Attempting PDF compilation. Reticulating splines etc...

@whedon
Copy link
Author

whedon commented Jul 10, 2018

@marcoalopez
Copy link

@whedon generate pdf

@whedon
Copy link
Author

whedon commented Jul 10, 2018

Attempting PDF compilation. Reticulating splines etc...

@whedon
Copy link
Author

whedon commented Jul 10, 2018

@marcoalopez
Copy link

@arfon @lheagy it seems that everything is ok now.

@lheagy
Copy link
Member

lheagy commented Jul 10, 2018

Thanks @marcoalopez. I have taken a quick look at GrainSizeTools and want to raise a couple things before we get started on the review. The package layout does not follow standard python packaging practices (see for example: https://packaging.python.org/tutorials/packaging-projects/).

/grain_size_tools  # repo
   /grain_size_tools  # python module
       __init__.py  # contains import to be brought in when the user runs import grain_size_tools
       tools.py
       piezometers.py
       ...

This would allow more flexible use - for example if a user wants to install your package through pypi and import it into a Jupyter notebook as a part of their workflow.

Is this something you would like to look into before we bring reviewers on board? If not, then I would recommend starting an issue if this is something you plan to do, or if not, then there should be some explanation in the readme about why the code is structured the way it is.

Please let me know how you would like to proceed. Thanks!

@marcoalopez
Copy link

Hello @lheagy. I was unaware of standard Python packaging practices. My intention is to follow standard Python practices, so I want to follow these guidelines. So for me is ok if you start an issue indicating this.

All the best, Marco

@lheagy
Copy link
Member

lheagy commented Jul 16, 2018

@marcoalopez: do you have any recommendations for reviewers?

@marcoalopez
Copy link

@lheagy: not really since I don't know anyone who works with grain size distributions or paleopiezometry who programs in Python. I know some people who work on similar topics but they program in Matlab.

@lheagy
Copy link
Member

lheagy commented Jul 19, 2018

👋 Hi @adebytes, @khaors: would you be willing to review this submission for JOSS?

@khaors
Copy link

khaors commented Jul 19, 2018

@lheagy Sure, I can help with the review.

@lheagy
Copy link
Member

lheagy commented Jul 19, 2018

Excellent, thanks @khaors!
👋 @jsta would you be willing to help with this review?

@lheagy
Copy link
Member

lheagy commented Jul 27, 2018

Hi @marcoalopez, @khaors: quick update, I have sent an email to @jsta asking if he is willing to review. I will keep you updated

@jsta
Copy link
Member

jsta commented Jul 30, 2018

@lheagy Yes, I can help with this review.

@lheagy
Copy link
Member

lheagy commented Jul 30, 2018

Excellent, thanks @jsta!

@lheagy
Copy link
Member

lheagy commented Jul 30, 2018

@whedon add @jsta as reviewer

@whedon
Copy link
Author

whedon commented Jul 30, 2018

OK, @jsta is now a reviewer

@lheagy
Copy link
Member

lheagy commented Jul 30, 2018

@whedon start review

@whedon
Copy link
Author

whedon commented Jul 30, 2018

OK, I've started the review over in #863. Feel free to close this issue now!

@lheagy lheagy closed this as completed Jul 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants