Re: [Bioc-devel] IRanges - PartitioningByEnd not found when package method runs.

2016-01-14 Thread Michael Lawrence
Thanks for the correction. I was starting to think my memory was
deteriorating, because I couldn't remember fixing that ;)

On Thu, Jan 14, 2016 at 3:43 PM, Hervé Pagès  wrote:
> On 01/13/2016 07:28 PM, Hervé Pagès wrote:
>>
>> Hi Ben, Martin,
>>
>> On 01/13/2016 08:19 AM, Morgan, Martin wrote:
>
> [...]
>>>
>>> So this is a bug in S4Vectors, but fixed in Bioc-devel where new
>>> package development should be occurring.
>>>
>>> I'll leave it to Herve or others to decide whether S4Vectors in
>>> release should be patched.
>>
>>
>> Michael fixed this in devel. Thanks Michael!
>
>
> I was told off-list that Jim Hester fixed this in devel. True:
>
> pages@latitude:~/svn/bioconductor/Rpacks/S4Vectors$ svn log -r 109840
> 
> r109840 | j.hester | 2015-10-22 09:56:54 -0700 (Thu, 22 Oct 2015) | 2 lines
>
> Use fully qualified names for IRanges::PartitioningByEnd
>
> 
>
> Sorry for that. You've got to give the right credit to the right
> people. So thanks Jim! But also thanks Michael for many other fixes
> to S4Vectors!
>
> H.
>
>
> --
> Hervé Pagès
>
> Program in Computational Biology
> Division of Public Health Sciences
> Fred Hutchinson Cancer Research Center
> 1100 Fairview Ave. N, M1-B514
> P.O. Box 19024
> Seattle, WA 98109-1024
>
> E-mail: hpa...@fredhutch.org
> Phone:  (206) 667-5791
> Fax:(206) 667-1319
>
> ___
> Bioc-devel@r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/bioc-devel

___
Bioc-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/bioc-devel

Re: [Bioc-devel] IRanges - PartitioningByEnd not found when package method runs.

2016-01-14 Thread Hervé Pagès

On 01/13/2016 07:28 PM, Hervé Pagès wrote:

Hi Ben, Martin,

On 01/13/2016 08:19 AM, Morgan, Martin wrote:

[...]

So this is a bug in S4Vectors, but fixed in Bioc-devel where new
package development should be occurring.

I'll leave it to Herve or others to decide whether S4Vectors in
release should be patched.


Michael fixed this in devel. Thanks Michael!


I was told off-list that Jim Hester fixed this in devel. True:

pages@latitude:~/svn/bioconductor/Rpacks/S4Vectors$ svn log -r 109840

r109840 | j.hester | 2015-10-22 09:56:54 -0700 (Thu, 22 Oct 2015) | 2 lines

Use fully qualified names for IRanges::PartitioningByEnd



Sorry for that. You've got to give the right credit to the right
people. So thanks Jim! But also thanks Michael for many other fixes
to S4Vectors!

H.

--
Hervé Pagès

Program in Computational Biology
Division of Public Health Sciences
Fred Hutchinson Cancer Research Center
1100 Fairview Ave. N, M1-B514
P.O. Box 19024
Seattle, WA 98109-1024

E-mail: hpa...@fredhutch.org
Phone:  (206) 667-5791
Fax:(206) 667-1319

___
Bioc-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/bioc-devel

Re: [Bioc-devel] IRanges - PartitioningByEnd not found when package method runs.

2016-01-14 Thread Morgan, Martin
Hi Ben --

R has a 'y' release in x.y.z every year, but Bioconductor has a .y release 
(where current devel becomes release) every 6 months.

Obviously we want to 'devel' bioc packages so that they can be released to 
users of what will be the release version of R at the time of release.

When the next (typically mid-April, but no date set yet) .y releases of R and 
Bioc coincide, Bioc-devel is based on R-devel.

When the next (typically mid-October) .y release of Bioc corresponds to no 
change in R's y, Bioc-devel is based on release R.

So yes, now you should be using R-devel. After mid-April, you'd use R-release 
for bioc devel. Then after mid-October, it would be back to R-devel.

That's not quite correct, because R has a release branch where bug fixes and 
minor '.z' changes come from. So from mid-April to mid-October, technically 
devel is against the R-x-y-branch of R svn repository.

This information has been added to the web site (I'm pretty sure it's already 
there, but I can't find it!) at

http://bioconductor.org/developers/how-to/useDevel/

The update will propagate in the next 20 minutes or so.

Martin
 

From: Ben Ward (TGAC) [ben.w...@tgac.ac.uk]
Sent: Thursday, January 14, 2016 4:20 AM
To: Hervé Pagès; Morgan, Martin; bioc-devel@r-project.org
Subject: RE: [Bioc-devel] IRanges - PartitioningByEnd not found when package 
method runs.

Hi, Herve, Martin,

Thanks! I've changed the function to use IntegerList instead of list in the 
branch, although at the moment it still fails at the same place.

Should I switch entirely to a devel R and bioc whilst I work on my package? You 
said 'during the current release cycle that means using the devel version on 
R', does this mean it switches and sometimes release R should be used?

Many thanks,
Ben.

From: Hervé Pagès [hpa...@fredhutch.org]
Sent: 14 January 2016 03:28
To: Morgan, Martin; Ben Ward (TGAC); bioc-devel@r-project.org
Subject: Re: [Bioc-devel] IRanges - PartitioningByEnd not found when package 
method runs.

Hi Ben, Martin,

On 01/13/2016 08:19 AM, Morgan, Martin wrote:
> Hi Ben -- nice to see you over here ;)
>
> You're using the release version of R (R-3.2.3) and presumably release 
> versions of packages (my advice on placing sessionInfo() was a little too 
> clever; you want your package loaded, so the versions of it's dependencies 
> are apparent, so in a separate R maybe library(testRpkg); sessionInfo(); I'll 
> update my support site post...).
>
> At least for Bioconductor packages, you want to be using the 'devel' version 
> of Bioconductor, and during the current release cycle that means the devel 
> version of R.
>
> Nonetheless, I started R-release, loaded the test package, ran the function, 
> and looked at traceback()
>
>> library(testRpkg)
>> doit()
> Error in end(PartitioningByEnd(x)) :
>error in evaluating the argument 'x' in selecting a method for function 
> 'end': Error: could not find function "PartitioningByEnd"
>> traceback()
> 9: end(PartitioningByEnd(x))
> 8: .unlist_NL_subscript(i, x)
> 7: fast_path_FUN(x, i)
> 6: subset_List_by_List(x, i)
> 5: object[index]
> 4: object[index]
> 3: subsetSites(dna, 1:50)
> 2: subsetSites(dna, 1:50)
> 1: doit()
>
> The suspect looks like .unlist_NL_subscript, so I went looking for it
>
>> getAnywhere(.unlist_NL_subscript)
> A single object matching '.unlist_NL_subscript' was found
> It was found in the following places
>namespace:S4Vectors
> with value
>
> function (i, x)
> {
>  offsets <- c(0L, end(PartitioningByEnd(x))[-length(x)])
>  i <- i + offsets
>  unlist(i, use.names = FALSE)
> }
> 
>
> So S4Vectors wants to use PartitioningByEnd. I looked at it's NAMESPACE 
> imports (actually I looked in the SVN repository...) for the packages it 
> imports from...
>
>> names(getNamespaceImports("S4Vectors"))
> [1] "base" "methods"  "utils""stats""stats4"
> [6] "BiocGenerics"
>
> and find that it does not import IRanges. So the problem is a bug in 
> S4Vectors.
>
> I don't see the problem in bioc-devel, and to see why I had to be a bit 
> clever -- there is no error, so no opportunity to use traceback(). Instead, I 
> used the advanced-but-flexible 'trace' function to set a tracer on 
> PartioningByEnd, where the tracer is the 'recover' function...
>
>> trace(IRanges::PartitioningByEnd, tracer=recover)
>
> So when PartioningByEnd is evalutated, R will get me the 'recover' prompt...
>> doit()
> Tracing IRanges::PartitioningByEnd(x) on entry
>
> Enter a frame number, or 0 to exit
>
>   1: doit()
>   2: subsetSites(dna, 1:50)
>   3: subsetSites(dna, 1:50)
>   4: object[index]
>   5: object[index]
>   6: subset_List_by_List(x, i)
>   7: fast_path_FUN(x, i)
>   8: .unlist_NL_subscript(i, x)
>   9: end(IRanges::PartitioningByEnd(x))
> 10: IRanges::PartitioningByEnd(x)
>
> Selection:
>
> This shows me the call stack (looks almost identical, so I could have 
> 'guessed

Re: [Bioc-devel] IRanges - PartitioningByEnd not found when package method runs.

2016-01-14 Thread Ben Ward (TGAC)
Hi, Herve, Martin,

Thanks! I've changed the function to use IntegerList instead of list in the 
branch, although at the moment it still fails at the same place. 

Should I switch entirely to a devel R and bioc whilst I work on my package? You 
said 'during the current release cycle that means using the devel version on 
R', does this mean it switches and sometimes release R should be used?

Many thanks,
Ben.

From: Hervé Pagès [hpa...@fredhutch.org]
Sent: 14 January 2016 03:28
To: Morgan, Martin; Ben Ward (TGAC); bioc-devel@r-project.org
Subject: Re: [Bioc-devel] IRanges - PartitioningByEnd not found when package 
method runs.

Hi Ben, Martin,

On 01/13/2016 08:19 AM, Morgan, Martin wrote:
> Hi Ben -- nice to see you over here ;)
>
> You're using the release version of R (R-3.2.3) and presumably release 
> versions of packages (my advice on placing sessionInfo() was a little too 
> clever; you want your package loaded, so the versions of it's dependencies 
> are apparent, so in a separate R maybe library(testRpkg); sessionInfo(); I'll 
> update my support site post...).
>
> At least for Bioconductor packages, you want to be using the 'devel' version 
> of Bioconductor, and during the current release cycle that means the devel 
> version of R.
>
> Nonetheless, I started R-release, loaded the test package, ran the function, 
> and looked at traceback()
>
>> library(testRpkg)
>> doit()
> Error in end(PartitioningByEnd(x)) :
>error in evaluating the argument 'x' in selecting a method for function 
> 'end': Error: could not find function "PartitioningByEnd"
>> traceback()
> 9: end(PartitioningByEnd(x))
> 8: .unlist_NL_subscript(i, x)
> 7: fast_path_FUN(x, i)
> 6: subset_List_by_List(x, i)
> 5: object[index]
> 4: object[index]
> 3: subsetSites(dna, 1:50)
> 2: subsetSites(dna, 1:50)
> 1: doit()
>
> The suspect looks like .unlist_NL_subscript, so I went looking for it
>
>> getAnywhere(.unlist_NL_subscript)
> A single object matching '.unlist_NL_subscript' was found
> It was found in the following places
>namespace:S4Vectors
> with value
>
> function (i, x)
> {
>  offsets <- c(0L, end(PartitioningByEnd(x))[-length(x)])
>  i <- i + offsets
>  unlist(i, use.names = FALSE)
> }
> 
>
> So S4Vectors wants to use PartitioningByEnd. I looked at it's NAMESPACE 
> imports (actually I looked in the SVN repository...) for the packages it 
> imports from...
>
>> names(getNamespaceImports("S4Vectors"))
> [1] "base" "methods"  "utils""stats""stats4"
> [6] "BiocGenerics"
>
> and find that it does not import IRanges. So the problem is a bug in 
> S4Vectors.
>
> I don't see the problem in bioc-devel, and to see why I had to be a bit 
> clever -- there is no error, so no opportunity to use traceback(). Instead, I 
> used the advanced-but-flexible 'trace' function to set a tracer on 
> PartioningByEnd, where the tracer is the 'recover' function...
>
>> trace(IRanges::PartitioningByEnd, tracer=recover)
>
> So when PartioningByEnd is evalutated, R will get me the 'recover' prompt...
>> doit()
> Tracing IRanges::PartitioningByEnd(x) on entry
>
> Enter a frame number, or 0 to exit
>
>   1: doit()
>   2: subsetSites(dna, 1:50)
>   3: subsetSites(dna, 1:50)
>   4: object[index]
>   5: object[index]
>   6: subset_List_by_List(x, i)
>   7: fast_path_FUN(x, i)
>   8: .unlist_NL_subscript(i, x)
>   9: end(IRanges::PartitioningByEnd(x))
> 10: IRanges::PartitioningByEnd(x)
>
> Selection:
>
> This shows me the call stack (looks almost identical, so I could have 
> 'guessed' that I wanted to look at .unlist_NL_subscript again) and suggests 
> that the code has been fixed to explicitly indicate where PartitioningByEnd 
> comes from. This is confirmed by looking at the code definition
>
>> getAnywhere(".unlist_NL_subscript")
> A single object matching '.unlist_NL_subscript' was found
> It was found in the following places
>namespace:S4Vectors
> with value
>
> function (i, x)
> {
>  offsets <- c(0L, end(IRanges::PartitioningByEnd(x))[-length(x)])
>  i <- i + offsets
>  unlist(i, use.names = FALSE)
> }
> 
>
> So this is a bug in S4Vectors, but fixed in Bioc-devel where new package 
> development should be occurring.
>
> I'll leave it to Herve or others to decide whether S4Vectors in release 
> should be patched.

Michael fixed this in devel. Thanks Michael! I should probably backport
the fix to release.

@Ben: FWIW subsetting with an IntegerList subscript is much more
efficient than with an ordinary list:

 index <- rep.int(IntegerList(index), length(object))
 object[index]

It will make a big difference (e.g. 100x faster or more) if 'object' is
a DNAStringSet with tens of thousands of sequences.

Cheers,
H.

>
> Thanks for the bug report!
>
> Martin
> 
> From: Bioc-devel [bioc-devel-boun...@r-project.org] on behalf of Ben Ward 
> (TGAC) [ben.w...@tgac.ac.uk]
> Sent: Wednesday, January 13, 2016 10:57 A