qiime2 / q2-longitudinal

QIIME 2 plugin for paired sample comparisons
BSD 3-Clause "New" or "Revised" License
9 stars 18 forks source link

volatility: metadata column names containing periods cannot be interpreted #133

Open nbokulich opened 5 years ago

nbokulich commented 5 years ago

Bug Description If metadata column names contain periods, that column cannot be viewed on the volatility plot. It seems that the periods may be interpreted as special characters.

Steps to reproduce the behavior see here (sourced from forum xref below)

Expected behavior Those columns should be viewable in the volatility plot.

References forum xref

thermokarst commented 5 years ago

My guess is that some vega lookup is using the Javascript dot accessor (data.peanut), when it should be using the square bracket lookup (data['peanut']). The problem here is when you have a period in the key: data.peanut.butter ({data: { peanut: { butter: 'yummy!' } } }) vs data['peanut.butter'] ({data: { 'peanut.butter': 'peanut butter jelly time!' } }).

David-Rod commented 5 years ago

Hey there @nbokulich and @thermokarst,

After reviewing this issue with @ebolyen and @Oddant1, it seems that a newer version of Vega has solved the issue. The Vega version used by the user was version 3.3.1. The current version in the QIIME 2 source code is version 4.4.0. I replicated this issue (as closely as I could) with current files from the tutorial, and Q2 View has no issue displaying the plot for group column names containing a..

TLDR: I think we can close this issue.

Thanks.

nbokulich commented 5 years ago

thanks for investigating @David-Rod !

Maybe we should a) pin a minimum vega version (if it is not already) and b) maybe also toss in a quick unit test? (though if we are positive this is related to the vega version then no need for a test). Interested to hear @thermokarst 's thoughts.