On 09/30/2016 11:44 PM, Robert M. Flight wrote:
Regarding Cytoscape, at least, I know RCytoscape and categoryCompare use
active Cytoscape sessions during vignette builds, but the build
maintainers need to know about it. I also don't know if the review
builds would have access to that.
Yes, this is one of those cases where the additional infrastructure to
support an active Cytoscape session adds complexity to the
administration of the build system. Looking forward, I don't think new
packages should rely on this kind of service during package build and
check. Rather, the package should check for appropriate pre-conditions
to the functions that they implement. This is a challenge for all
heavily visual applications.
Martin
Robert
On Fri, Sep 30, 2016, 9:53 PM Vincent Carey <[email protected]
<mailto:[email protected]>> wrote:
i would have comments on StarTrek. if you would like me to be
reviewer of
that i can do it.
a query on policy
how do you propose to handle needlessly repeated code?
the code given below is repeated many many times with slight
variations for
selected KEGG_path. i would ask the author
to implement DRY (don't repeat yourself), find a functional approach
to the
repeated element, and
simplify. i can imagine that damping enthusiasm of the contributor
but I
think it is worth holding back
until such a thing is accomplished. too meddlesome?
if (KEGG_path=="Carb_met") {
pathways.list <- keggList("pathway", "hsa")## returns the list of human
pathways
mer<-select_path_carb(Carbohydrate)
common<-intersect(pathways.list,mer)
lo<-list()
for (i in 1:length(pathways.list)){
if (length(intersect(pathways.list[[i]],common)!=0)){
lo[[i]]<-pathways.list[[i]]
names(lo)[[i]]<-names(pathways.list)[[i]]
}
}
pathways.list<-lo[lapply(lo,length)!=0]
pathway.codes <- sub("path:", "", names(pathways.list))
a<-do.call("rbind", pathways.list)
}
Another issue the author seems to raise is the need for "open cytoscape
session" for meaningful tests.
I think this will never happen on the build system. So the author
has to
emulate the cytoscape behavior
perhaps with some static objects that cytoscape might return on
interaction.
On Fri, Sep 30, 2016 at 5:22 PM, Martin Morgan <
[email protected]
<mailto:[email protected]>> wrote:
> On 09/30/2016 12:05 PM, Michael Lawrence wrote:
>
>> I have been chipping in here and there, but I'm not sure if I'm
>> causing more confusion than help. I just select packages that
interest
>> me and contribute bite size pieces, but I would be happy to assume a
>> more formal role. It takes very little of my time.
>>
>> One suggestion is that reviews should be incremental, i.e., provide
>> obvious feedback first, let the package authors respond, then dig
>> deeper for more detailed comments.
>>
>
> Michael, I've found your comments / reviews really helpful. They are
> generally on-target and constructive, and help spur the authors to
think
> about what they are doing. They do in some ways 'confuse' the
issue (how is
> the package author to know that the 'assignee' is the official
reviewer?
> are the people commenting on the package providing a consistent
message?),
> but definitely a plus. I encourage others to participate in the public
> review process in this way, too.
>
> It would be great to hear from people who would like to take on
the role
> of assigned reviewer. I'd suggest you identify package(s) that
would be
> interesting to you, and send email to me. I'll arrange for you to
become
> the assigned reviewer, and provide basic instructions and
guidelines. In
> the future, you will be able to take on a package more directly.
>
> From the formal review process, incremental additional comments
are not a
> good way to go, because the package author can feel like they are on a
> never-ending treadmill of requirements. Instead I think the formal
review
> should more or less set out a contract -- do these things and your
package
> will meet reviewer requirements for inclusion in Bioconductor.
This makes
> it harder / more time consuming to review the package, because
there is
> only the one chance to identify the most important points. Also,
one needs
> to follow through and confirm that yes, the appropriate changes
were made.
>
> It will be interesting to adjust the public review process, especially
> after the next release. For instance, one approach I like is to
provide a
> review with editable check boxes, with the package author editing the
> review with information about commits, etc, that satisfy the
concerns being
> expressed. I don't really like the canned check-list in the joss
link Tim
> mentions; they point to important areas but seem too generic to
lead to
> constructive changes to individual packages.
>
> Martin
>
>
>> Michael
>>
>> On Fri, Sep 30, 2016 at 8:49 AM, Kasper Daniel Hansen
>> <[email protected]
<mailto:[email protected]>> wrote:
>>
>>> One of the advantages of the Github review process is open
review. We
>>> have
>>> some amount of packages submitted in the last moment (and I am
guilty of
>>> one of these). Is help with the reviews needed? If so, how do
we know
>>> where to concentrate?
>>>
>>> Kasper
>>>
>>> [[alternative HTML version deleted]]
>>>
>>> _______________________________________________
>>> [email protected] <mailto:[email protected]>
mailing list
>>> https://stat.ethz.ch/mailman/listinfo/bioc-devel
>>>
>>
>> _______________________________________________
>> [email protected] <mailto:[email protected]>
mailing list
>> https://stat.ethz.ch/mailman/listinfo/bioc-devel
>>
>>
>
> This email message may contain legally privileged
and/or...{{dropped:2}}
>
>
> _______________________________________________
> [email protected] <mailto:[email protected]> mailing
list
> https://stat.ethz.ch/mailman/listinfo/bioc-devel
>
--
Vincent J. Carey, Ph.D.
Professor of Medicine
Channing Division of Network Medicine
Brigham and Women's Hospital
Harvard Medical School
181 Longwood Avenue
Boston, MA 02115 USA
[[alternative HTML version deleted]]
_______________________________________________
[email protected] <mailto:[email protected]> mailing list
https://stat.ethz.ch/mailman/listinfo/bioc-devel
This email message may contain legally privileged and/or...{{dropped:2}}
_______________________________________________
[email protected] mailing list
https://stat.ethz.ch/mailman/listinfo/bioc-devel
