这是indexloc提供的服务，不要输入任何密码
Skip to content

minor tmap_animation update suggestion #220

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 5, 2018
Merged

minor tmap_animation update suggestion #220

merged 1 commit into from
Jul 5, 2018

Conversation

jannes-m
Copy link
Contributor

@jannes-m jannes-m commented Jul 4, 2018

Hey Martijn,
I am just playing around with tmap_animation(). There is a minor probably Windows-only issue related to:
https://stackoverflow.com/questions/40096966/r-tmap-animation-tmap-imagemagick
When installing ImageMagick, one has to check a box "install legacy files (e.g., convert.exe)". Not doing so results in an error message since there seems to be another convert command under Windows. This this is not well-documented, I think it would be more user-friendly to be more specific in the code of tmap_animation() in the sense to make clear that the convert command of ImageMagick should be used (see minor file change).
As a side note, the first example of tmap_animation() is not working, since there is no object called NLD_prov.

@jannes-m
minor tmap_animation update suggestion
e6f236f 
Hey Martijn,
I am just playing around with `tmap_animation()`. There is a minor probably Windows-only issue related to: 
https://stackoverflow.com/questions/40096966/r-tmap-animation-tmap-imagemagick
When installing ImageMagick, one has to check a box "install legacy files (e.g., convert.exe)". Not doing so results in an error message since there seems to be another convert command under Windows. This this is not well-documented, I think it would be more user-friendly to be more specific in the code of `tmap_animation()` in the sense to make clear that the convert command of ImageMagick should be used (see minor file change).
As a side note, the first example of `tmap_animation()` is not working, since there is no object called `NLD_prov`.
@mtennekes mtennekes merged commit c17bbdf into r-tmap:master Jul 5, 2018
@mtennekes
Copy link
Member

Thx! Please check if your email is correct in the DESCRIPTION file.

It doesn't work at my laptop (Ubuntu 16.04), i.e. "magick" is not known:

d> system("magick convert -version")
sh: 1: magick: not found
d> system("convert -version")
Version: ImageMagick 6.9.7-4 Q16 x86_64 20170114 http://www.imagemagick.org
Copyright: © 1999-2017 ImageMagick Studio LLC
License: http://www.imagemagick.org/script/license.php
Features: Cipher DPC Modules OpenMP 
Delegates (built-in): bzlib djvu fftw fontconfig freetype jbig jng jpeg lcms lqr ltdl lzma openexr pangocairo png tiff wmf x xml zlib

I'm not sure if this issue Ubuntu/Linux general, or just my laptop. Also, I'm not sure what works on MacOS.

Is the old code syscall(paste("convert -delay ", delay, " ", d, "/*.png \"", filename, "\"", sep="")) not working on Windows, or why did you put "magick" in front?

@jannes-m
Copy link
Contributor Author

jannes-m commented Jul 5, 2018 •
edited
Loading

Hey Martijn,
well, the code works if one has clicked the checkbox "install legacay files" during the installation process. If one hasn't done so, it doesn't work unless you use "magick" in front of magick commands. Seems like a Windows-only problem. So I would suggest to either add a Windows if-clause to the code or to say explicitly in the documentation that one has to install the legacy files.

@mtennekes
Copy link
Member

Ok, so the safest option is to create a Windows-only clause. So then users won't have to tick the "install legacy files" options?

@jannes-m
Copy link
Contributor Author

jannes-m commented Jul 5, 2018

Exactly!

@Nowosad Nowosad mentioned this pull request Jul 8, 2018
Closed
mtennekes added a commit that referenced this pull request Jul 8, 2018
@mtennekes
fixed and enhanced tmap_animation (#220, #224), added JSS code vignette
d251aa3
mtennekes added a commit that referenced this pull request Jul 10, 2018
@mtennekes
fixed tmap_animation again (#220 #224)
dffe089
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jannes-m @mtennekes