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

Accept hyphen for coord ranges. #1102

Merged
merged 7 commits into from Jul 13, 2018

Conversation

Projects
None yet
3 participants
@nathanhaigh
Copy link
Contributor

nathanhaigh commented Jul 10, 2018

This PR satisfies #1100 by making the following changes to JBrowse behaviour:

  • Accept hyphens (-) as the basepair range separator. This is in addition to the currently used ..
  • There is flexibility in the number of hyphens accept in the input, in case of typos.
  • Made the hyphen (-) the canonical basepair range separator. The legacy .. is still accepted.

nathanhaigh added some commits Jul 10, 2018

Support the use of a hyphen as a coordinate range separator.
Also split the regex over multiple lines to allow for easier to follow documentation.
@nathanhaigh

This comment has been minimized.

Copy link
Contributor Author

nathanhaigh commented Jul 10, 2018

I have some questions about the level of flexibility currently allowed for the coordinate specification - is it needed?

@rbuels rbuels added this to the 1.15.0 milestone Jul 10, 2018

@nathanhaigh

This comment has been minimized.

Copy link
Contributor Author

nathanhaigh commented Jul 10, 2018

Reverted the last commit which changed the displayed coordinate range separator from .. to -. I'll introduce that proposed change in a new PR.

As this PR stands, it simply introduces - as a possible coordinate range separator.

@cmdcolin

This comment has been minimized.

Copy link
Contributor

cmdcolin commented Jul 11, 2018

This looks good to me, did a pull and it keeps the .. in the location bar but allows - to be used

@rbuels

This comment has been minimized.

Copy link
Collaborator

rbuels commented Jul 11, 2018

I feel like the pendulum is swinging toward '-' being canonical, let's see in a day or two.

In the meantime, @nathanhaigh, more locstring parsing tests would be awesome! Like seq names with colons and seq names with hyphens. :-)

@nathanhaigh

This comment has been minimized.

Copy link
Contributor Author

nathanhaigh commented Jul 12, 2018

There is still an outstanding issue here. That is JBrowse does not switch to reference sequences with colons when their name is entered into the location search box. I don't think this is to do with the locstring parsing since tests show the expected behaviour for strings without basepair coordinates (single point location or a range) return null.

Perhaps someone with more familiarity with JBrowse (@cmdcolin @rbuels @keiranmraine ) could take a look? I've added some test data for this (see tests/data/obscure_contig_names/).

@rbuels rbuels merged commit 66f8b91 into GMOD:dev Jul 13, 2018

1 check passed

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

@nathanhaigh nathanhaigh deleted the CroBiAd:accept_hyphen_coord_ranges branch Jul 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.