search

lawremi / rtracklayer

R interface to genome annotation files and the UCSC genome browser
Other
28 stars 17 forks source link

BiocIO additional updates #34

Closed mtmorgan closed 3 years ago

mtmorgan commented 3 years ago

The intention of the pull request is to clean up addition issues related to creation of the BiocIO package.

https://github.com/lawremi/rtracklayer/commit/561cc87b59da5872a76595398a77def19141d9cb is initial clean-up reduces some of the warnings, unrelated to BiocIO.

https://github.com/lawremi/rtracklayer/commit/d2956a64d53204c7dc3a8ffaf6473e95c55c87fc removes the CompressedFile class definition, even though we would have liked to keep it to signal a warning about deprecation; having identical class names in BiocIO and rtacklayer seems to cause problems as documented in the commit message.

https://github.com/lawremi/rtracklayer/commit/ec57795c8c222d41839dabb2cf9f9c00ca3e7557 replaces RTLFile with BiocFile in a couple of places in the code and documentation.

The last commit https://github.com/lawremi/rtracklayer/commit/47f3bfb759d63d443dcedefd87fb145fa83b1f0e may need some attention -- it reverts a portion of a commit from a few days ago that nonetheless breaks, e.g., ?TrackHubGenome.

A version bump is required.

hpages commented 3 years ago

@mtmorgan @lawremi Can we make sure that important/breaking changes like this PR (and other recent important changes like commit 688b8b7c84925057b48851fc75ae52709f383741) are accompanied by a version bump and actually propagate? Otherwise it makes it really difficult for the maintainers of affected packages to reproduce and troubleshoot. I just spent the last 20 min. trying to reproduce https://bioconductor.org/checkResults/3.13/bioc-LATEST/SplicingGraphs/ until I finally discovered that my rtracklayer 1.51.0 was not the same as the rtracklayer 1.51.0 used by the build system :-(

Thanks!

lawremi commented 3 years ago

I wonder if I messed up the merge. Somehow the Bioc and GitHub histories diverged, and I got totally confused and issued commands at random until things were consistent. Did I accidentally revert one of the commits, like 064ae63?

mtmorgan commented 3 years ago

It's hard to tell; I investigated a bit and I think https://github.com/lawremi/rtracklayer/pull/26/commits/65f32a5b12d0af71533ef5d366d2330f0a7a621b (in Daniel's original pull request) introduced the change incorporated as https://github.com/lawremi/rtracklayer/commit/064ae63f8a15b282c48ae2776eafa444328f34b4 so I don't think you were responsible for that...

lawremi commented 3 years ago

Ok, thanks for the updates. I bumped the version as @hpages suggested (thanks for the reminder!). Also, a heads up to @sanchit-saini that the track hub stuff might be broken now.