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

Fix font problems in devteam/xy_plot tool #71

Closed
jennaj opened this issue Dec 18, 2017 · 18 comments
Closed

Fix font problems in devteam/xy_plot tool #71

jennaj opened this issue Dec 18, 2017 · 18 comments
Assignees
Labels
bug fixathon 06/18 Fixathon June 2018 test/retest-pass passed retest

Comments

@jennaj
Copy link
Member

jennaj commented Dec 18, 2017

MTS: https://toolshed.g2.bx.psu.edu/view/devteam/xy_plot/ecb437f1d298

Tool: Plotting Tool for multiple series and graph types | 1.0.2

Used in this GTN tutorial, section "Hands-on: Plot Rarefaction": http://galaxyproject.github.io/training-material/topics/metagenomics/tutorials/mothur-miseq-sop/tutorial.html

@jennaj
Copy link
Member Author

jennaj commented Feb 14, 2018

ping @davebx

@martenson
Copy link
Member

updated on Main in 34a4dd4

@jennaj jennaj mentioned this issue Mar 28, 2018
57 tasks
@jennaj
Copy link
Member Author

jennaj commented May 16, 2018

Retesting

@jennaj jennaj added install usegalaxy.org tool install usegalaxy.org requested test/retest-do active tests and removed help wanted issue pending assignee or working group labels May 16, 2018
@nekrut nekrut added the fixathon 06/18 Fixathon June 2018 label Jun 4, 2018
@martenson martenson removed the install usegalaxy.org tool install usegalaxy.org requested label Jun 4, 2018
@jennaj
Copy link
Member Author

jennaj commented Jun 6, 2018

@martenson @natefoo version 1.0.2 fails.

Does this look like a Galaxy config problem or a tool problem?

Dataset Error
An error occured while running the tool toolshed.g2.bx.psu.edu/repos/devteam/xy_plot/XY_Plot_1/1.0.2.

Tool execution generated the following messages:

Fontconfig warning: "/etc/fonts/fonts.conf", line 146: blank doesn't take any effect anymore. please remove it from your fonts.conf

@jennaj jennaj added test/retest-fail failed retest and removed test/retest-do active tests labels Jun 6, 2018
@jennaj jennaj assigned martenson and unassigned jennaj Jun 6, 2018
@martenson
Copy link
Member

martenson commented Jun 6, 2018

@jennaj can you please share the history?

edit: found it in the bug report

from @nekrut:

We should remove this tool. It was a proof of concept tool James wrote during an Encode meeting in 2008. New charts can do the same thing. a.

@martenson
Copy link
Member

martenson commented Jun 6, 2018

I propose hiding the tool and adjusting the tutorial to use charts.

@natefoo
Copy link
Member

natefoo commented Jun 6, 2018

This can be fixed relatively easily by using exit codes instead of stderr.

The warning is caused by conda using a newer version of fontconfig than the one on the system, for which /etc/fonts/fonts.conf is written. There's a suitable <env>/etc/fonts/fonts.conf in the conda env but I can't think of a way to get $FONTCONFIG_FILE set properly for this.

@martenson
Copy link
Member

@natefoo the actual error is different (can be seen in the history: https://usegalaxy.org/histories/view?id=223db61e77830a8e).

Error in Summary.factor(c(1L, 2L, 49L, 50L, 51L, 52L, 53L, 54L, 55L, 56L,  : 
  ‘range’ not meaningful for factors

@natefoo
Copy link
Member

natefoo commented Jun 6, 2018

That's hid 516, hid 517 was successful though.

@martenson
Copy link
Member

Oh so you think 516 had bad inputs? 517 is definitely just warning written to stderr, I can see the plot fine.

@natefoo
Copy link
Member

natefoo commented Jun 6, 2018

Well, interesting development... fontconfig in the R conda env is correctly reading its own fonts.conf. But when we run jobs, we set $FONTCONFIG_FILE to /cvmfs/main.galaxyproject.org/fontconfig/fonts.conf, which loads some custom fonts in /cvmfs/main.galaxyproject.org/fontconfig/fonts and then includes /etc/fonts/fonts.conf. This was done long, long ago for pre-TS tools.

If I don't set $FONTCONFIG_FILE, it'll probably fix this tool, but I don't know what tool(s) used these fonts and whether they will break if these fonts aren't available. If I knew what tools they were, I could set $FONTCONFIG_FILE just for these tools.

@natefoo
Copy link
Member

natefoo commented Jun 6, 2018

However, I am inclined to just unset this and fix any old tools it breaks as they are discovered. Scatterplot and Histogram are the likely subjects.

@natefoo
Copy link
Member

natefoo commented Jun 6, 2018

Well this is interesting. I ran Plotting tool, Scatterplot, and Histogram, then disabled $FONTCONFIG_FILE and ran all 3 again. With $FONTCONFIG_FILE set, Plotting tool ends in error but produces a valid plot:

before

With it unset, Plotting tool ends ok but the fonts in the plot are symbol fonts:

after

Unsetting it had no effect on Scatterplot or Histogram.

I think maybe the tool should add a requirement on some font package in conda?

@martenson martenson changed the title Upgrade Plotting Tool version 1.0.1 to 1.0.2. Used in tutorials (v 1.0.1 does not work with it) Fix font problems in devteam/xy_plot tool Jun 6, 2018
@jennaj
Copy link
Member Author

jennaj commented Jun 6, 2018

test history (late, you found it in other ticket): https://usegalaxy.org/u/jen/h/16s-microbial-analysis-with-mothur---proccess

Dataset 517 is in an error state (red). The graph does look how I would expect it to (but can't really double check, am comparing with whatever tool version was used to create tutorial graphics, this has never been on main before and haven't checked at eu).

@jennaj
Copy link
Member Author

jennaj commented Jun 6, 2018

@natefoo right, that was the problem before - red error dataset but graphics produced. If you create a ticket for a tool update, could you link it back? That might be a less risky choice (adding conda dep to tool) rather than making a change that impacts all tools.

@jennaj
Copy link
Member Author

jennaj commented Jun 6, 2018

yes, hid 516 failed because of usage - The way the tool form is designed, each series input needs to have the "Header in first line?" set to yes individually. This is easy to miss, especially when doing that step in the tutorial where you add in 10+ series, each its own section. The form could be modified or the stderr could be more informative (if can't parse the first line, fail but warn user "Do the inputs have headers? Currently set as "No" for series N."

But considering how much other stuff is going on, is minor, so didn't report it, just the hid 517 failure. I'll deal with that later, especially if users start to report running into it (expect they will, students/new users use these tutorials).

@jennaj jennaj added bug and removed enhancement labels Jun 6, 2018
@natefoo
Copy link
Member

natefoo commented Jun 6, 2018

I've fixed this by installing DejaVu fonts into CVMFS and making fontconfig aware of them with $XDG_DATA_HOME. The process was:

  1. cvmfs_server transaction $repo && mkdir -p /cvmfs/$repo/xdg/data/fonts
  2. curl -L http://sourceforge.net/projects/dejavu/files/dejavu/2.37/dejavu-fonts-ttf-2.37.tar.bz2 | tar jxf - -C /cvmfs/$repo/xdg/data/fonts
  3. Unset $FONTCONFIG_FILE and set XDG_DATA_HOME=/cvmfs/$repo/xdg/data in 502a0b3

(where $repo is test.galaxyproject.org or main.galaxyproject.org)

@natefoo natefoo closed this as completed Jun 6, 2018
@jennaj
Copy link
Member Author

jennaj commented Jun 6, 2018

thanks for testing, also worked for me. changing test status.

@jennaj jennaj added test/retest-pass passed retest and removed test/retest-fail failed retest labels Jun 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fixathon 06/18 Fixathon June 2018 test/retest-pass passed retest
Projects
None yet
Development

No branches or pull requests

4 participants