[Bioc-devel] Best Practices for Renaming S4 Slots
The reason I mentioned the API freeze was to emphasize that we are at a stage in our release schedule where we must be very careful to not make changes that are potentially disruptive. I'm sorry if this suggested that renaming a slot is changing the API. Let's be very clear on this: it's not. However, in my experience, changing the internals of an S4 class tends to be disruptive. Not from an end-user point of view (they're supposed to use accessor functions), but from the point of view of other packages that rely on serialized objects (either from their own data/ folder or from an experiment package) for their examples, vignettes, and/or unit tests. These packages will break and their maintainers will need some time to identify the problem and address it. So I stand to my initial recommendation to not do this now but to wait for the new BioC 3.15 development cycle. It's only 2 weeks away so it will delay Chris by 2 weeks only. Cheers, H.
On 15/10/2021 12:21, Vincent Carey wrote:
I find your arguments fairly reasonable.? I wonder if you have been
through the process of restoring
data from an object serialized with the old slot names,?using the
revised package.? Maybe it is not even a use case for your
packages.? If you have worked through the implications of this change
for all dependent packages,
and since the policy on slot names is not clearly articulated, I would
be inclined to allow the change -- but I would like
to hear from other folks on bioc-devel and in our core.
On Fri, Oct 15, 2021 at 1:51 PM Chris Eeles
<christopher.eeles at outlook.com <mailto:christopher.eeles at outlook.com>>
wrote:
Hi Vincent,
Thanks for sharing your thoughts.____
__ __
I guess my thinking was that the entire point of using S4 classes
and OOP is having accessor methods to provide an implementation
independent API. In the Bioconductor guidelines it specifically
tells users not to access slots using the `@` operator, as an
implementation change in the class may break any scripts doing so.
Therefore, my changing slot names should have no effect on users
following the Bioconductor coding best practices, assuming I
maintain the old accessors methods with a .Deprecation warning, as
per the cited guideline. That was indeed my plan.
So I would argue that no, class definitions are not part of the API,
especially if I am just renaming slots. Indeed isn?t that one of the
supposed strengths of OOP programming and the use of interfaces?
Obviously I have already agreed to wait for 3.15 to make the
changes, but I do not think it is clear from the current guidelines
that deprecation rules apply to slots. Given that `@` isn?t even a
generic, there would be no way to send a message to the user except
through the accessor methods, which they would never see if they
weren?t already using the accessor API. So for users accessing data
via `@`, the deprecation guidelines provide no benefits because they
failed to follow the best practices.
My opinion of the developer-user contract for S4 classes is that the
API would not change without due warning, and if implementation
really is independent of interface, then any changes made to an S4
class should be fine, so long as all the original methods still work
and can be deprecated according to the cited guidelines.
Additionally, if changes to a class require so much work, it
incentives developers to simply ditch old S4 classes and reimplement
them in a new package. Doesn?t that go against the spirit of reuse
that is supposed to be encouraged by adoption of S4 classes?
TL;DR ? IMO API = interface; implementation is developer business
Best,____
Chris____
__ __
*From:*Vincent Carey <stvjc at channing.harvard.edu
<mailto:stvjc at channing.harvard.edu>>
*Sent:* October 15, 2021 1:31 PM
*To:* Chris Eeles <christopher.eeles at outlook.com
<mailto:christopher.eeles at outlook.com>>
*Cc:* Herv? Pag?s <hpages.on.github at gmail.com
<mailto:hpages.on.github at gmail.com>>; bioc-devel at r-project.org
<mailto:bioc-devel at r-project.org>
*Subject:* Re: [Bioc-devel] Best Practices for Renaming S4 Slots____
__ __
I will defer to Herve about all details, but I would say that this
level of change control is implied by the "no changes____
to package API without an interval of deprecation spanning at least
one release".? See
https://bioconductor.org/developers/how-to/deprecation/
<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbioconductor.org%2Fdevelopers%2Fhow-to%2Fdeprecation%2F&data=04%7C01%7C%7C8ec56a4ffda74802e7ab08d990019bc7%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637699158846578705%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=1vILqDKSUJPMJ5gKZ1y%2Ftlf8rKtrZmX6tL6xGTINEo8%3D&reserved=0>____
That text mentions that removal may take 18 months. ____
__ __
Whatever is exposed cannot be changed without a deprecation period,____
in which the functionality is preserved, but notification is given
to users/developers, through .Deprecated, that____
functionality will change and advice is given on the alternate
approach to be used.____
__ __
Is a slot name part of the API?? It isn't completely obvious, but in
the case of serialized objects, it turns out that it is.____
I don't know that our guidelines have sufficient details on this
process, but we welcome your input on where to____
best outline/advertise this.____
__ __
On Fri, Oct 15, 2021 at 1:22 PM Chris Eeles
<christopher.eeles at outlook.com
<mailto:christopher.eeles at outlook.com>> wrote:____
Message received. I will leave that branch for later. Is this
information available on the Bioconductor website at all? It
would have been useful to find out sooner.
Best,
Chris
-----Original Message-----
From: Bioc-devel <bioc-devel-bounces at r-project.org
<mailto:bioc-devel-bounces at r-project.org>> On Behalf Of Chris Eeles
Sent: October 15, 2021 1:10 PM
To: Herv? Pag?s <hpages.on.github at gmail.com
<mailto:hpages.on.github at gmail.com>>; bioc-devel at r-project.org
<mailto:bioc-devel at r-project.org>
Subject: Re: [Bioc-devel] Best Practices for Renaming S4 Slots
Thanks Herve,
I actually got the updateObject method working after sending
this email, but thanks for the information. Maybe it is worth
adding a section on this topic to the Bioconductor developer
section?
Unfortunately, I was unaware that the start of development cycle
was the best time to implement this change. I am currently
planning to have this done for the 3.14 release.
I am introducing new accessors as well but keeping the old ones
for backwards compatibility using aliases.
How discouraged are slot name changes in a release? A lot of the
changes on our road map require the slots to be renamed so it
would significantly delay required features if I were to wait.
I plan to put in the work so that those using accessors
shouldn't notice a difference.
Best,
Chris
-----Original Message-----
From: Herv? Pag?s <hpages.on.github at gmail.com
<mailto:hpages.on.github at gmail.com>>
Sent: October 15, 2021 12:39 PM
To: Chris Eeles <christopher.eeles at outlook.com
<mailto:christopher.eeles at outlook.com>>;
bioc-devel at r-project.org <mailto:bioc-devel at r-project.org>
Subject: Re: [Bioc-devel] Best Practices for Renaming S4 Slots
Hi Chris,
There was some formatting issues with my previous answer so I'm
sending it again. Hopefully this time the formatting is better.
See below.
On 14/10/2021 13:08, Chris Eeles wrote:
> Hello BioC community,
>
> I am the lead developer for the CoreGx, PharmacoGx, RadioGx
and ToxicoGx packages. We have recently been working on major
updates to the structure of a CoreSet, which is inherited by the
main class in all three of the other packages listed.
>
> While making these changes, we would like to rename some
CoreSet slots to increase the amount of code that can be
refactored into CoreGx, simplifying maintenance and development
of inheriting packages in the future.
>
> The slot names and their accessors will be made more generic,
for example the 'cell' slot will become 'sample' to allow a
CoreSet derived class to store Biological model systems other
than cancer cell lines. Similarly, 'drug' or 'radiation' slots
in inheriting packages will be replaced with a 'treatment' slot
in the CoreSet. This will allow us to simplify inheritance,
removing much redundant code from the inheriting packages and
setting us up to integrate other lab packages, such as Xeva for
PDX models, into the general CoreSet infrastructure.
>
> I took a brief look through the Bioconductor developer
documentation but didn't see anything talking about best
practices for renaming slots.
>
> It is easy enough to make the code changes, but my major
concern is being able to update existing objects from these
packages to use the new slot names.
>
> I am aware of the updateObject function in BiocGenerics, but
tried using it to update some example data in CoreGx without
success.
>
> Is this the proper function to use when you wish to update an
S4 object whose class definition has been modified? If not, is
there existing infrastructure for accomplishing this task?
Yes updateObject() is the proper function to use but the
function has no way to guess how to fix your objects. The way
you tell it what to do is by implementing methods for your objects.
For example if you renamed the 'cell' slot -> 'sample', your
updateObject() method will be something like this:
setMethod("updateObject", "CoreSet",
? ? ?function(object, ..., verbose=FALSE)
? ? ?{
? ? ? ? ?## The "cell" slot was renamed -> "sample" in
CoreGx_1.7.1.
? ? ? ? ?if (.hasSlot(object, "cell")) {
? ? ? ? ? ? ?object <- new("CoreSet",
? ? ? ? ? ? ? ? ? ? ? ? ? ?sensitivity=object at sensitivity,
? ? ? ? ? ? ? ? ? ? ? ? ? ?annotation=object at annotation,
?molecularProfiles=object at molecularProfiles,
? ? ? ? ? ? ? ? ? ? ? ? ? ?sample=object at cell,
? ? ? ? ? ? ? ? ? ? ? ? ? ?datasetType=object at datasetType,
? ? ? ? ? ? ? ? ? ? ? ? ? ?perturbation=object at perturbation,
? ? ? ? ? ? ? ? ? ? ? ? ? ?curation=object at curation)
? ? ? ? ? ? ?return(object)
? ? ? ? ?}
? ? ? ? ?object
? ? ?}
)
The best time to do this internal renaming is at the beginning
of the BioC 3.15 development cycle (i.e. right after the 3.14
release).
If in the future, other slots get renamed or added, you'll need
to modify the updateObject() method above like this:
setMethod("updateObject", "CoreSet",
? ? ?function(object, ..., verbose=FALSE)
? ? ?{
? ? ? ? ?## The "cell" slot was renamed -> "sample" in
CoreGx_1.7.1.
? ? ? ? ?if (.hasSlot(object, "cell")) {
? ? ? ? ? ? ?object <- new("CoreSet",
? ? ? ? ? ? ? ? ? ? ? ? ? ?sensitivity=object at sensitivity,
? ? ? ? ? ? ? ? ? ? ? ? ? ?annotation=object at annotation,
?molecularProfiles=object at molecularProfiles,
? ? ? ? ? ? ? ? ? ? ? ? ? ?sample=object at cell,
? ? ? ? ? ? ? ? ? ? ? ? ? ?datasetType=object at datasetType,
? ? ? ? ? ? ? ? ? ? ? ? ? ?perturbation=object at perturbation,
? ? ? ? ? ? ? ? ? ? ? ? ? ?titi=object at curation)? # use "titi"
here too!
? ? ? ? ? ? ?return(object)
? ? ? ? ?}
? ? ? ? ?## The "curation" slot was renamed -> "titi" in
CoreGx_1.9.1.
? ? ? ? ?if (.hasSlot(object, "curation")) {
? ? ? ? ? ? ?object <- new("CoreSet",
? ? ? ? ? ? ? ? ? ? ? ? ? ?sensitivity=object at sensitivity,
? ? ? ? ? ? ? ? ? ? ? ? ? ?annotation=object at annotation,
?molecularProfiles=object at molecularProfiles,
? ? ? ? ? ? ? ? ? ? ? ? ? ?sample=object at sample,? # use
"sample" here!
? ? ? ? ? ? ? ? ? ? ? ? ? ?datasetType=object at datasetType,
? ? ? ? ? ? ? ? ? ? ? ? ? ?perturbation=object at perturbation,
? ? ? ? ? ? ? ? ? ? ? ? ? ?titi=object at curation)
? ? ? ? ? ? ?return(object)
? ? ? ? ?}
? ? ? ? ?object
? ? ?}
)
And so on...
Hope this helps,
H.
>
> Any tips for implementing slot renaming, as well as links to
existing documentation or articles on the topic would be
appreciated.
>
> Best,
> ---
> Christopher Eeles
> Software Developer
> BHK
>
Laboratory<https://na01.safelinks.protection.outlook.com/?url=http%3A% <https://na01.safelinks.protection.outlook.com/?url=http%3A%25>
> 2F%2Fwww.bhklab.ca
<https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2F2fwww.bhklab.ca%2F&data=04%7C01%7C%7C8ec56a4ffda74802e7ab08d990019bc7%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637699158846588662%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=oQjaDi%2By2aBcR0fDoAq%2FsGw92T4NQKENJF6RX8zb9AA%3D&reserved=0>%2F&data=04%7C01%7C%7C170e009bdbda4e7f182308d990
>
000468%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637699152031882488
>
%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6I
>
k1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=eu7jm6LiWISZ01etllrdipTAkVtI4a7
> rZumELnt0siI%3D&reserved=0> Princess Margaret Cancer
>
Centre<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F% <https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%25>
> 2Fwww.pmgenomics.ca
<https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2F2fwww.pmgenomics.ca%2F&data=04%7C01%7C%7C8ec56a4ffda74802e7ab08d990019bc7%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637699158846588662%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=dEu%2Bv1UzsrTRLmjw5bPtEhHEGnYYmGQN%2B9a6PjLLj2E%3D&reserved=0>%2Fpmgenomics%2F&data=04%7C01%7C%7C170e009bdbda
>
4e7f182308d990000468%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C6376
>
99152031882488%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2l
>
uMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=g8gXrDgyJ5YRHdiBv
> q77WiDkCgWcYhWWW9R7OWKMxqw%3D&reserved=0>
> University Health
>
Network<https://na01.safelinks.protection.outlook.com/?url=http%3A%2F% <https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%25>
> 2Fwww.uhn.ca
<https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2F2fwww.uhn.ca%2F&data=04%7C01%7C%7C8ec56a4ffda74802e7ab08d990019bc7%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637699158846598621%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=muzG7Yz1KliU9n%2FNpQXkfj4oNpJqFnnrPlD6ae81BAk%3D&reserved=0>%2F&data=04%7C01%7C%7C170e009bdbda4e7f182308d990000468
>
%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637699152031882488%7CUnk
>
nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWw
>
iLCJXVCI6Mn0%3D%7C1000&sdata=IJKtucTmctj0z227jGpqnBeZ0D9ruvODNLkmT
> QBdX%2Bs%3D&reserved=0>
>
>
>? ? ? ?[[alternative HTML version deleted]]
>
> _______________________________________________
> Bioc-devel at r-project.org <mailto:Bioc-devel at r-project.org>
mailing list
>
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fstat <https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fstat>.
> ethz.ch
<https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fethz.ch%2F&data=04%7C01%7C%7C8ec56a4ffda74802e7ab08d990019bc7%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637699158846608572%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=MbzF83IFpbNGcb9HFG8q6TnPUeCfeXB8HU%2BsDscmuCo%3D&reserved=0>%2Fmailman%2Flistinfo%2Fbioc-devel&data=04%7C01%7C%7C170e00
>
9bdbda4e7f182308d990000468%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%
>
7C637699152031882488%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQI
>
joiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=nNq88Xhpby%
> 2FVJxZ%2BdBWPk72Cp%2FS3EsaFgQ3FhrkaaH4%3D&reserved=0
>
--
Herv? Pag?s
Bioconductor Core Team
hpages.on.github at gmail.com <mailto:hpages.on.github at gmail.com>
_______________________________________________
Bioc-devel at r-project.org <mailto:Bioc-devel at r-project.org>
mailing list
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fstat.ethz.ch%2Fmailman%2Flistinfo%2Fbioc-devel&data=04%7C01%7C%7C170e009bdbda4e7f182308d990000468%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637699152031882488%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=nNq88Xhpby%2FVJxZ%2BdBWPk72Cp%2FS3EsaFgQ3FhrkaaH4%3D&reserved=0
<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fstat.ethz.ch%2Fmailman%2Flistinfo%2Fbioc-devel&data=04%7C01%7C%7C8ec56a4ffda74802e7ab08d990019bc7%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637699158846608572%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=zNC1AVCelVynnCTWncoEeARzFx%2B%2BX3PKQ0WO%2Fp5X%2F5w%3D&reserved=0>
_______________________________________________
Bioc-devel at r-project.org <mailto:Bioc-devel at r-project.org>
mailing list
https://stat.ethz.ch/mailman/listinfo/bioc-devel
<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fstat.ethz.ch%2Fmailman%2Flistinfo%2Fbioc-devel&data=04%7C01%7C%7C8ec56a4ffda74802e7ab08d990019bc7%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637699158846618526%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=uyYpbCJlKPCS3EQVZR8vEQaRj6btOUkJXs2qOOCWonU%3D&reserved=0>____
The information in this e-mail is intended only for the person to
whom it is
addressed. If you believe this e-mail was sent to you in error and
the e-mail
contains patient information, please contact the
Partners?Compliance?HelpLine at
http://www.partners.org/complianceline
<https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.partners.org%2Fcomplianceline&data=04%7C01%7C%7C8ec56a4ffda74802e7ab08d990019bc7%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637699158846618526%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=fxCSoQPYoR9LPKcF%2BW6Khuk4LHgJ%2B37%2B5mNDwKFustQ%3D&reserved=0>?.
If the e-mail was sent to you in error
but does not contain patient information, please contact the sender
and properly
dispose of the e-mail.____
The information in this e-mail is intended only for th...{{dropped:18}}