[Bioc-devel] Best Practices for Renaming S4 Slots
Vincent Carey
@tvjc @end|ng |rom ch@nn|ng@h@rv@rd@edu
Fri Oct 15 21:21:58 CEST 2021
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>
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>
> *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>
> 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> On Behalf Of Chris
> Eeles
> Sent: October 15, 2021 1:10 PM
> To: 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
>
> 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>
> Sent: October 15, 2021 12:39 PM
> To: Chris Eeles <christopher.eeles using outlook.com>; 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 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
>
> _______________________________________________
> 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 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 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
<http://www.partners.org/complianceline> . 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.
[[alternative HTML version deleted]]
More information about the Bioc-devel
mailing list