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

bed_sort can't sort bed_intersect results #134

Closed
jayhesselberth opened this Issue Nov 1, 2016 · 3 comments

Comments

Projects
None yet
1 participant
@jayhesselberth
Collaborator

jayhesselberth commented Nov 1, 2016

because they have start.x instead of start. Think about how to do this the right way. Making start a param (with default value start) might be helpful.

@jayhesselberth

This comment has been minimized.

Collaborator

jayhesselberth commented Nov 10, 2016

Best would be to provide a spec that could be passed to dplyr::arrange.

@jayhesselberth

This comment has been minimized.

Collaborator

jayhesselberth commented Dec 1, 2016

Could also just pass an NSE spec in with dots; would probably simplify the TRUE/FALSE params.

@jayhesselberth

This comment has been minimized.

Collaborator

jayhesselberth commented Dec 1, 2016

Can do away with current bed_sort if we put these in a sorting example somewhere:

  • arrange(x, start)
  • arrange(x, desc(start))
  • arrange(x, chrom, start)
  • mutate(x, .size = end - start) %>% arrange(.size)

Note that arrange ignores groups, so group_by(x, chrom) %>% arrange(start) is not the same as arrange(chrom, start).

This would address the parent problem, by requiring an explicit sorting step (e.g., you would have to arrange(start.x).

The only remaining reason to keep bed_sort() is to use the is_sorted internal function, which is currently used by merge, coverage and closest. Not sure whether it's worth keeping it around, given it is not exported.

jayhesselberth added a commit that referenced this issue Dec 1, 2016

deprecate bed_sort() (closes #134)
- use dplyr::arrange() instead

@wafflebot wafflebot bot added the in progress label Dec 1, 2016

@wafflebot wafflebot bot removed the in progress label Dec 2, 2016

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