[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