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 Request: Rougier #28

Closed
wants to merge 40 commits into
base: master
from

Conversation

@rougier
Member

rougier commented Feb 26, 2017

AUTHOR

Dear @ReScience/editors,

I request a review for the following replication:

Original article

Title: Weighted Voronoi Stippling
Author(s): Adrian Secord
Journal (or Conference): International Symposium on Non-photorealistic Animation and
Rendering
Year: 2002
DOI: 10.1145/508530.508537
PDF: http://mrl.nyu.edu/~ajsecord/npar2002/npar2002_ajsecord_preprint.pdf

Replication

Author(s): Nicolas P. Rougier
Repository: https://github.com/rougier/ReScience-submission/tree/rougier-2017
PDF: https://github.com/rougier/ReScience-submission/blob/rougier-2017/article/Rougier-2017.pdf
Keywords: Python, Stippling, Voronoi, Computer Graphics, Blue Noise
Language: Python
Domain: Signal Processing

Results

  • Article has been fully replicated
  • Article has been partially replicated
  • Article has not been replicated

Potential reviewers


EDITOR

  • Editor acknowledgment (@ThomasA) February 27, 2017
  • Reviewer 1 (@rth) February 28, 2017
  • Reviewer 2 (@almarklein) February 27, 2017
  • Review 1 decision [accept] April 19, 2017
  • Review 2 decision [accept] April 28, 2017
  • Editor decision [accept] May 9, 2017
@rougier

This comment has been minimized.

Member

rougier commented Feb 26, 2017

@khinsen Can you handle this submission (from me) ? I think @ThomasA can edit it but I'll let you decide. I'll now restrict myself to my author role for this submission.

@rougier rougier changed the title from Rougier 2017 to Review Request: Rougier 2017 Feb 26, 2017

@rougier rougier changed the title from Review Request: Rougier 2017 to Review Request: Rougier Feb 26, 2017

@khinsen

This comment has been minimized.

khinsen commented Feb 27, 2017

Yes, I'll take over the EIC role for this submission!

@ThomasA can you supervise the reviewing process?

@ThomasA

This comment has been minimized.

ThomasA commented Feb 27, 2017

Yes 👍
Just a moment while I read up on the procedure as this is my first.

@ThomasA

This comment has been minimized.

ThomasA commented Feb 27, 2017

@rth and @soolijoo can you review this replication?

@soolijoo

This comment has been minimized.

Contributor

soolijoo commented Feb 27, 2017

Hi, Sorry this is outside my expertise.

@ThomasA

This comment has been minimized.

ThomasA commented Feb 27, 2017

@soolijoo OK, I will try someone else.

@ThomasA

This comment has been minimized.

ThomasA commented Feb 27, 2017

@almarklein can you review this replication?

@almarklein

This comment has been minimized.

almarklein commented Feb 27, 2017

Sure! Is this ready for review at this point?

@ThomasA

This comment has been minimized.

ThomasA commented May 10, 2017

@rougier I am stuck trying to build the PDF. I cannot use the 'fontawesome' package on my system. XeLaTeX seems to try to call some font-generating stuff that fails. I have TeXlive 2016 installed in Ubuntu (installed directly from TexLive - not Ubuntu repositories). The fontawesome package is installed, but somehow not useable.

@ThomasA

This comment has been minimized.

ThomasA commented May 10, 2017

@rougier if I comment out \usepackage{fontawesome} from the template, make just seems to stall and never get further than this:

$ make all
[1/4] Processing Rougier-2017.tex (pass 1)

ThomasA added some commits May 10, 2017

Article template updated from submission repo
rescience-template.tex was updated from ReScience/ReScience-submission.
@rougier

This comment has been minimized.

Member

rougier commented May 10, 2017

If you compile using directly the latex command, what is the output ?
The fontawesome package is needed only for putting the github glyph into the red box on the right.

@rougier

This comment has been minimized.

Member

rougier commented May 10, 2017

For the fontawesome installation, what is the error in the font generating stuff ?

@ThomasA

This comment has been minimized.

ThomasA commented May 10, 2017

@rougier By the way, I just by mistake pushed some updates to your repo. Now I do not understand how I actually had the permission to do so? I guess this is not a major problem - the updates are supposed to go in the archive repo anyway.

@ThomasA

This comment has been minimized.

ThomasA commented May 10, 2017

Regarding fonawesome, making the paper says:

$ make Rougier-2017.pdf
 [1/4] Processing Rougier-2017.tex (pass 1)

kpathsea: Running mktextfm FontAwesome
/usr/local/texlive/2016/texmf-dist/web2c/mktexnam: Could not map source abbreviation F for FontAwesome.
/usr/local/texlive/2016/texmf-dist/web2c/mktexnam: Need to update /usr/local/texlive/2016/texmf-dist/fonts/map/fontname/special.map?
mktextfm: Running mf-nowin -progname=mf \mode:=ljfour; mag:=1; nonstopmode; input FontAwesome
This is METAFONT, Version 2.7182818 (TeX Live 2016) (preloaded base=mf)


kpathsea: Running mktexmf FontAwesome
! I can't find file `FontAwesome'.
<*> ...our; mag:=1; nonstopmode; input FontAwesome
                                                  
Please type another input file name
! Emergency stop.
<*> ...our; mag:=1; nonstopmode; input FontAwesome
                                                  
Transcript written on mfput.log.
grep: FontAwesome.log: Ingen sådan fil eller filkatalog
mktextfm: `mf-nowin -progname=mf \mode:=ljfour; mag:=1; nonstopmode; input FontAwesome' failed to make FontAwesome.tfm.
kpathsea: Appending font creation commands to missfont.log.

I am whining about it here as well: https://tex.stackexchange.com/questions/369068/how-can-i-use-the-fontawesome-package-with-xelatex

@rougier

This comment has been minimized.

Member

rougier commented May 10, 2017

Yes, I can see your changes in my repo. That's totally weird.
In the meantime, once you've made all the changes, I'll be able to build the PDF and push it.

@rougier

This comment has been minimized.

Member

rougier commented May 10, 2017

Just to be sure, you've also installed the FontAwesome.otf somewhere ?

@ThomasA

This comment has been minimized.

ThomasA commented May 10, 2017

Yes, it is in the Texlive tree

@rougier

This comment has been minimized.

Member

rougier commented May 10, 2017

Oh, the font is bundled with the tex package ?
For the push permission, I'll ask github support how this is possible.

@ThomasA

This comment has been minimized.

ThomasA commented May 10, 2017

Yes, fontawesome is included in Texlive. I am getting some help here: https://tex.stackexchange.com/questions/369068/how-can-i-use-the-fontawesome-package-with-xelatex, but none of it seems to work...

@ThomasA

This comment has been minimized.

ThomasA commented May 10, 2017

OK, I finally made it work.
One remaining oddity is that xelatex does not seem to interpret `` '' correctly the way pdflatex does. Is that normal?

@ThomasA

This comment has been minimized.

ThomasA commented May 10, 2017

I have been looking at this: https://tex.stackexchange.com/questions/193412/what-is-happening-to-the-quotes and this: https://tex.stackexchange.com/questions/278251/ligatures-stopped-working-in-xelatex.
I can fix the quotes if I insert \defaultfontfeatures{Ligatures=TeX} after line 33 in 'rescience-template.tex', but only if I run \setmainfont{[some font]} afterwards to set a font. Without it it has no effect. If I attempt to insert the \defaultfontfeatures line before line 32 in 'rescience-template.tex', make stalls in the first processing of Rougier-2017.tex (doesn't err - just never finishes).
I suspect that fontawesome is somehow messing with this Ligatures feature...

@ThomasA

This comment has been minimized.

ThomasA commented May 10, 2017

Should we simply ignore that `` '' quotes look ugly with this compile workflow?

@rougier

This comment has been minimized.

Member

rougier commented May 15, 2017

Could you make a PR for the main rescience-template and fix it on this repo then ?
I go the explanation for your commit in my branch, it's a new feature of the PR where you can allow edits from maintainers (there is a tickbox for allowing or not).

@ThomasA

This comment has been minimized.

ThomasA commented May 18, 2017

Sorry, I got stuck last week trying to compile the paper for publishing. This week I am swamped with a course I am giving. I hope to pick this back up tomorrow or during the weekend.

@rougier

This comment has been minimized.

Member

rougier commented May 29, 2017

@ThomasA Any chance to publish it this week ?

@khinsen

This comment has been minimized.

khinsen commented May 31, 2017

@ThomasA a gentle reminder...

@ThomasA

This comment has been minimized.

ThomasA commented May 31, 2017

@ThomasA

This comment has been minimized.

ThomasA commented Jun 2, 2017

@rougier could you please rebuild the PDF on rougier/ReScience-submission on your rougier-2017 branch (including the changes it turned out I was able to push to your repo)? I have no XeTeX experience and seem unable to get the fonts set up correctly on my system to make it compile.
Then I will pull it from there and archive it.

@rougier

This comment has been minimized.

Member

rougier commented Jun 2, 2017

Done.

@ReScience ReScience locked and limited conversation to collaborators Jun 2, 2017

@ThomasA

This comment has been minimized.

ThomasA commented Jun 2, 2017

EDITOR

This submission has been published and will soon appear at http://rescience.github.io/read/

DOI

@ThomasA ThomasA closed this Jun 2, 2017

@khinsen

This comment has been minimized.

khinsen commented Jun 7, 2017

This looks good! Thanks to @ThomasA for handling the review, to @rth and @almarklein for reviewing, and to @rougier for doing the replication work!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.