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 for v1.12.3 defaultTracks #896

Merged
merged 1 commit into from Jan 30, 2018

Conversation

Projects
None yet
4 participants
@rdhayes
Contributor

rdhayes commented May 31, 2017

cmdcolin's suggested () addition to fix the test for defaultTracks in config as an array. fixes #892

JGI Compgen Web User
@cmdcolin

This comment has been minimized.

Contributor

cmdcolin commented May 31, 2017

I guess I feel like there needs to be a better solution.

This original problem seems like it happened if someone used a config like

"defaultTracks": ["track1","track2","track3"] 

instead of

"defaultTracks": "track1,track2,track3"

Of course, the comma separated list is what should be used, but we could support the array too. However, what this PR does is it just ignores the first one if it is used, so I think it's a little weird.

@rdhayes

This comment has been minimized.

Contributor

rdhayes commented May 31, 2017

This is the simple fix we discussed in #892. Support for arrays like that in JBrowse config (as opposed to Apollo) is a broader issue, no?

@cmdcolin

This comment has been minimized.

Contributor

cmdcolin commented May 31, 2017

As far as I could tell no code in Apollo touches the defaultTracks (you can search entire repo and I see no mention of defaultTracks) so basically, my only idea is that it was a simple misconfiguration. In that case, this fix basically is just a confusing workaround, and it doesn't really get applied across the board to other things like forceTracks and alwaysOnTracks, and basically seems like it should be reverted to the olden days (not checking for array at all)

@rdhayes

This comment has been minimized.

Contributor

rdhayes commented May 31, 2017

@erasche should we expect lists or arrays in config? There's no mention of the array structure syntax for parameters in the gmod.org docs (http://gmod.org/wiki/JBrowse_Configuration_Guide#General_configuration_options), only:

  • an array of values can be built up over multiple lines e.g.
[trackMetadata]
    sources =
       + data/mymeta.csv
       + data/more_meta.csv
@cmdcolin

This comment has been minimized.

Contributor

cmdcolin commented May 31, 2017

Arrays are certainly allowed in some places in the config, but the defaultTracks, forceTracks, and alwaysOnTracks are just comma separated lists of track labels. I imagine that is the way things are because &tracks= is also a comma separated list of track labels so it is just a consistent handling of all of those.

@rdhayes

This comment has been minimized.

Contributor

rdhayes commented Aug 15, 2017

Any additional comment on this one?
It's an important fix to make regardless of approach, as 1.12.3 as currently released will not display default track configuration.

@rbuels rbuels added this to the 1.12.4 milestone Jan 30, 2018

@rbuels rbuels merged commit 3a10216 into master Jan 30, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@wafflebot wafflebot bot removed the in progress label Jan 30, 2018

@rbuels rbuels deleted the defaultTracks_fix branch Feb 14, 2018

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