[Bioc-devel] Best Practices for Renaming S4 Slots

Hervé Pagès hp@ge@@on@g|thub @end|ng |rom gm@||@com
Fri Oct 15 21:32:34 CEST 2021


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 using outlook.com <mailto:christopher.eeles using 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 using channing.harvard.edu
>     <mailto:stvjc using channing.harvard.edu>>
>     *Sent:* October 15, 2021 1:31 PM
>     *To:* Chris Eeles <christopher.eeles using outlook.com
>     <mailto:christopher.eeles using outlook.com>>
>     *Cc:* 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____
> 
>     __ __
> 
>     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
>         <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 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}}



More information about the Bioc-devel mailing list