ReScience / submissions

ReScience C submissions
28 stars 7 forks source link

10 year challenge: Eglen and Wong (2008) #34

Closed sje30 closed 3 years ago

sje30 commented 4 years ago

Original article: https://arxiv.org/abs/1208.0986 (postprint of 2008 paper)

PDF URL: https://github.com/sje30/rescience-hor/blob/master/template-master/article2.pdf Metadata URL: https://github.com/sje30/rescience-hor/blob/master/template-master/metadata.yaml Code URL: https://github.com/sje30/rescience-hor/

Scientific domain: Computational Neuroscience Programming language: R Suggested editor: Timothée Poisot or Karthik Ram or Etienne Roesch

sje30 commented 4 years ago

Apologies in advance for grafting your pdf onto the front of my pdf -- I ran out of time to investigate if your article style could be used with Rmarkdown. So creating article2.pdf was my solution.

rougier commented 4 years ago

Thanks for your submission, we'll assign an editor soon. For the published version (if accepted), that would be great if you can use the template.

I'll edit your submission.

rougier commented 4 years ago

@thmosqueiro Can you review this submission for the Ten Years Reproducibility Challenge ?

sje30 commented 4 years ago

Thanks for your submission, we'll assign an editor soon. For the published version (if accepted), that would be great if you can use the template.

yes, of course, will be able to swtich to your template. I think I can just export the markdown to .tex and then fix up from there.

rougier commented 4 years ago

@rgutzen @junpenglao @rcaze @iampritishpatil Can you review thus submission for the Ten Years Reproducibility Challenge ?

rgutzen commented 4 years ago

Sorry, I'm afraid I have no experience with R.

rougier commented 4 years ago

@rguten Thanks for your answer. @junpenglao @rcaze @iampritishpatil @benoit-girard @rossant ca you reviwe this (short) paper?

iampritishpatil commented 4 years ago

Sorry I missed the tag last time. I don't think I'd manage well as I haven't used R in several years, and seems there are a non trivial amount of code and dependencies in R. I'm sorry.

rougier commented 4 years ago

@iampritishpatil No problem, thank for the answer.

rcaze commented 4 years ago

Sorry for not answering earlier, I just started to re-enabled notification on github. I'm afraid my knowledge of R is also insufficient.

benoit-girard commented 4 years ago

I haven't used R in the last 17 years, I fear I won't be able to examine the code...

rougier commented 4 years ago

@thmosqueiro @ChrisRackauckas Could you review this submission (https://github.com/ReScience/submissions/issues/34) ?

ChrisRackauckas commented 4 years ago

I am afraid I do not have the bandwidth right now to properly handle this! Sorry!

rougier commented 4 years ago

@ChrisRackauckas @benoit-girard Thanks for the answer. @eroesch stephanmg degoldschmidt Could you review this article (R + Computational Neuroscience) ?

rougier commented 4 years ago

Sorry for the delay. @eroesch @stephanmg @degoldschmidt Could you review this article (R + Computational Neuroscience) ?

rougier commented 3 years ago

Oh my god, I'm very late on following this submission. @ChrisRackauckas @thmosqueiro @eroesch @stephanmg @degoldschmidt Could you review this article (R + Computational Neuroscience) ?

stephanmg commented 3 years ago

Hi @rougier I could review it.

sje30 commented 3 years ago

Thank you both! I had forgotten about this. I just checked the repo, and it looks like the binder broke sometime in the last year (rmarkdown needs to be installed, and there is a small pandoc-citeproc issue). Can you give me the weekend to fix this, which in the interim will update it to R version 4.0 and handle the change in pandoc-citeproc which is now missing from Ubuntu (this will teach me not to pin all the versions down for mybinder!)

sje30 commented 3 years ago

The launch binder button on the readme of https://github.com/sje30/rescience-hor is working again; just needed a couple of tweaks to system R images that had changed in the last year.

stephanmg commented 3 years ago

Hi,

since this is my first review, can somebody give me some pointers for best practices on how the review should be conducted?

rougier commented 3 years ago

Sure, have a look at: https://rescience.github.io/edit/

Note that this submission is related to the "Ten Years Reproducibility Challenge" where authors try to reproduce (re-run) their own code. You can also browse open reviews on the same issue.

stephanmg commented 3 years ago

@sje30 I looked at your reproduction.

I have some suggestions for (minor) revision before accepting the submission: Figures:

Code:

Altogether great replication of figures and tables. This looks good!

Review criteria:

  1. Reproducibility of the replication: Yes, I can reproduce via Binder and on my computer.
  2. Clarity and completeness of the accompanying article: Great, everything ran OOTB for me.
  3. Clarity of the code: Okay.
  4. Clarity and completeness of the accompanying article: Okay.
sje30 commented 3 years ago

Thanks for doing this so promptly!

  • Figure 1: Original figure is black/white, I would avoid using red/green colors for the dots - will be much easier to discriminate for people with color vision deficiency.

The 'red' is actually 'orangered' for the exact reason you mention. I can change it to blue though if you prefer?

Code:

  • Error: Status code 504 returned - I assume this some Binder-related issue?

Hmm... the binder just worked for me now. Could you please try again?

  • hor_bdmin.R - contains some commented code which seems not necessary, can this be removed?

  • maps2.R - same as above I can certainly tidy it up! great idea.

  • Overall many empty lines and superfluous whitespace, could you correct this?

Is that in the R files?

Altogether great replication of figures and tables. This looks okay.

Thank you.

stephanmg commented 3 years ago

Thanks for doing this so promptly!

  • Figure 1: Original figure is black/white, I would avoid using red/green colors for the dots - will be much easier to discriminate for people with color vision deficiency. The 'red' is actually 'orangered' for the exact reason you mention. I can change it to blue though if you prefer?

Well, call me pedantic, I would just prefer black and white as in the original figure.

Code: - Error: Status code 504 returned - I assume this some Binder-related issue? Hmm... the binder just worked for me now. Could you please try again?

Will try again later.

  • hor_bdmin.R - contains some commented code which seems not necessary, can this be removed? - maps2.R - same as above I can certainly tidy it up! great idea.
  • Overall many empty lines and superfluous whitespace, could you correct this? Is that in the R files? Yes, in the R files, there is a lot of empty lines, sometimes in sequence. Altogether great replication of figures and tables. This looks okay. Thank you.

Besides this, I would suggest to tidy up the repository into code and markup if possible.

stephanmg commented 3 years ago

By the way: I am a little confused about the following: You write that the arXiv paper is a post print, do you mean preprint? Because I see your paper being published in Vis Neurosci. (and I would call this a post print) and arXiv is obviously a preprint server.

rougier commented 3 years ago

@sje30 Can you address the comments?

sje30 commented 3 years ago

Thanks for these. Bit behind, but yes, I hope to do these in the next few days... thanks for the comments.

sje30 commented 3 years ago

hi all, am now working through these.

Postprint == correct here, I think. I uploaded the article to arxiv long after the paper was accepted.

sje30 commented 3 years ago

I have reverted figure 1 to b/w, and removed excess comments.

I'm now getting problems however getting article.tex to compile. I think it may be my lack of python skills, but when I run make in the template_master folder, I get the following error:

make         
./yaml-to-latex.py -i metadata.yaml -o metadata.tex
Traceback (most recent call last):
  File "/home/stephen/proj/2020/rescience-hor/template-master/./yaml-to-latex.py", line 93, in <module>
    content = generate_latex_metadata(filename_in, article)
  File "/home/stephen/proj/2020/rescience-hor/template-master/./yaml-to-latex.py", line 10, in generate_latex_metadata
    "% DO NOT EDIT - automatically generated from {filename}\n\n"
AttributeError: 'str' object has no attribute 'name'
make: *** [Makefile:18: metadata.tex] Error 1

I did also have to fix article.py to specify which loader to use.

sje30 commented 3 years ago
Python 3.9.6 (default, Jun 30 2021, 10:22:16) 
[GCC 11.1.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import yaml
>>> print (yaml.__version__)
5.4.1
rougier commented 3 years ago

Can you check if you have the latest version of the article template (with yaml-to-latex.py)?

sje30 commented 3 years ago

thanks. I started over with a fresh template/ and that seemed to do the job nicely. I have now pushed the new version to github, and converted from the Rmarkdown to use the template. See: https://github.com/sje30/rescience-hor/blob/master/template/article.pdf

sje30 commented 3 years ago

Do I need to do anything on here to signal the paper is ready for checking again?

rougier commented 3 years ago

sorry for the delay. Articlel looks good to me and from the review by @stephanmg I think we can accept the paper.

I'll try to edit it today starting from your new version. Stay tuned.

rougier commented 3 years ago

I would need a software heritage ID. Can you save your repo at https://www.softwareheritage.org/save-and-reference-research-software/ and give me back the corresponding swid?

sje30 commented 3 years ago

Thanks @rougier but am having problems uploading to software heritage. https://twitter.com/StephenEglen/status/1428808288506945536?s=20

perhaps it is just Friday night and it will be better in the morning!

khinsen commented 3 years ago

Software heritage seems to have some technical issues at the moment, I didn't manage to save a GitHub repo yesterday. BTW, you don't need an account there, what you did should have been sufficient.

sje30 commented 3 years ago

Thanks Konrad. Seems to be a bit healthier this morning -- I can see this now:

https://archive.softwareheritage.org/browse/origin/directory/?origin_url=https://github.com/sje30/rescience-hor

sje30 commented 3 years ago

@rougier : is this what you need? swh:1:dir:6088b7312865b39a4a5f5af78556bf6da5fa6409;origin=https://github.com/sje30/rescience-hor;visit=swh:1:snp:a2df6c1cf91ec9a80d08026d7728a503eb64c7a6;anchor=swh:1:rev:0882985f8a0e3ea32f8898175460bacc8060b552

sje30 commented 3 years ago

SWH

rougier commented 3 years ago

Yes, sorry for the delay. I'll try to do it today!

rougier commented 3 years ago

The sandboxed version is online at https://sandbox.zenodo.org/record/908095, can you check if everything's ok? Then I'll proceed with the actual publication.

sje30 commented 3 years ago

Super, thanks @rougier -- all looks good to me.

rougier commented 3 years ago

It's online at https://zenodo.org/record/5347786 ! Congratulations. It should appear soon on rescience website (a few minutes)

sje30 commented 3 years ago

Thanks! Great job.

rougier commented 3 years ago

And so sorry for the very long delay until publication.

sje30 commented 3 years ago

no problem at all!