[Bioc-devel] Best Practices for Renaming S4 Slots
Chris Eeles
chr|@topher@ee|e@ @end|ng |rom out|ook@com
Fri Oct 15 19:51:51 CEST 2021
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 using channing.harvard.edu>
Sent: October 15, 2021 1:31 PM
To: Chris Eeles <christopher.eeles using outlook.com>
Cc: Herv� Pag�s <hpages.on.github using gmail.com>; bioc-devel using 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 using outlook.com<mailto:christopher.eeles using 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 using r-project.org<mailto:bioc-devel-bounces using r-project.org>> On Behalf Of Chris Eeles
Sent: October 15, 2021 1:10 PM
To: Herv� Pag�s <hpages.on.github using gmail.com<mailto:hpages.on.github using gmail.com>>; bioc-devel using r-project.org<mailto:bioc-devel using 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 using gmail.com<mailto:hpages.on.github using gmail.com>>
Sent: October 15, 2021 12:39 PM
To: Chris Eeles <christopher.eeles using outlook.com<mailto:christopher.eeles using outlook.com>>; bioc-devel using r-project.org<mailto:bioc-devel using 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 using sensitivity,
annotation=object using annotation,
molecularProfiles=object using molecularProfiles,
sample=object using cell,
datasetType=object using datasetType,
perturbation=object using perturbation,
curation=object using 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 using sensitivity,
annotation=object using annotation,
molecularProfiles=object using molecularProfiles,
sample=object using cell,
datasetType=object using datasetType,
perturbation=object using perturbation,
titi=object using 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 using sensitivity,
annotation=object using annotation,
molecularProfiles=object using molecularProfiles,
sample=object using sample, # use "sample" here!
datasetType=object using datasetType,
perturbation=object using perturbation,
titi=object using 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 using r-project.org<mailto:Bioc-devel using r-project.org> mailing list
> 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 using gmail.com<mailto:hpages.on.github using gmail.com>
_______________________________________________
Bioc-devel using r-project.org<mailto:Bioc-devel using 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 using r-project.org<mailto:Bioc-devel using 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 ...{{dropped:19}}
More information about the Bioc-devel
mailing list