Repository navigation
Clobber & Overwrite? #5644
Description
Activity
Note: Implemented in #5171. That PR also depended on #5214 and #5492.
Re: Policy. IMHO, I think it was right to put that API change in 1.3 without backporting. At least, that was my understanding of how Astropy versioning works.
In regards your needs for backward compatibility in your affiliated package, for now, you can either:
- Catch and suppress the warning while continue to use
clobber; or - Check for Astropy version and use
clobber/overwriteas appropriate
- Catch and suppress the warning while continue to use
I must admit neither of @pllim's options are particularly attractive, but when you are trying to support astropy 1.0.* as well as 1.3, I don't quite see how it is possible to backport, since the one thing you may not be able to count on is for people to have necessarily updated 1.0 to its latest point release, so if you switch to
overwriteyou may break things regardless. I don't quite know how to best solve it. Maybe such deprecations should only happen in a long-term support releases? In some sense that may be more useful for people who only use LTS; that way we minimize the amount of breakage when they update to the next LTS.- Reacted by P. L. Lim
I think this is worth broader discussion, so I send a note to astropy-dev.
Reacted by P. L. LimI ended up writing compatibility functions to avoid these annoying warnings: https://git-cral.univ-lyon1.fr/MUSE/mpdaf/blob/master/lib/mpdaf/tools/astropycompat.py#L30
Having to use these functions everywhere is not practical, but having to check Astropy's version everywhere is not better.I wonder if in retrospective (something that could still be fixed in a bugfix release) we should have introduced
overwrite, changed all the docs, but only deprecated after a couple of releases where both options co-existed.This does seem to be causing a lot of hassle.. another example: sherpa/sherpa#328
So what about just removing the deprecation warning in 1.3.1 and re-introducing it in 2.0 LTS?
Reacted by Tom Aldcroft, Brigitta Sipőcz, P. L. Lim, Adam Ginsburg, Simon Conseil, Mike Jarvis and Russell OwenSee astropy/astropy-APEs#20 for an addendum to APE2 about formalising this process.
This issue of clobber vs. overwrite causes more pain than it solves real problems. Now that I need to take care of this in an STScI package - I hate this change. It is inconceivable that both
clobberandoverwriteare not coexisting for the period of deprecation especially considering how often this appears in code dependent on astropy.I like this suggestion #5171 (comment) but as an alternative, I would like to suggest something as simple as:
# OLD def writeto(self, fileobj, output_verify='exception', clobber=False, checksum=False):
could have been replaced with:
# NEW AND IMPROVED: def writeto(self, fileobj, output_verify='exception', overwrite=None, checksum=False, clobber=None): """ writeto(self, fileobj, output_verify='exception', overwrite=False, checksum=False) """ # Above, in the docstring I hid 'clobber' and described default overwrite value as False if overwrite is None: if clobber is None: overwrite = False else: # issue deprecation warning overwrite = clobber else: if clobber is not None: raise ValueError("clobber must be None when overwrite is set") # ..............
I'll submit a PR to hide the deprecation warning in the decorator. Sorry, I had a lot of things on my plate in the last few weeks and should've done this much earlier.
@MSeifert04 The bigger issue than warnings themselves is the fact that this change was not made "backward compatible" (in the sense that if I modify our code to use overwrite it will not work with astropy versions < 2 (or 1.3 or whatever)) unless I'll do some ugly dancing similar to pyspeckit/pyspeckit#207
Everyone... @MSeifert04 has reverted the warning in #5761 and the warning will "disappear" in 1.3.1 release (thank you!). So, I am closing this issue now.
However, be advised that the warning will come back in v2.0 (#5773) and deprecated codes be completely removed in v3.0 (#5774).
Just FWIW, if other people are struggling with wrappers etc., I dealt with the change like this (in
__init__.py), which also works with thewritetomethod:if 'clobber' in inspect.getargspec(astropy.io.fits.writeto).args: arg_overwrite = 'clobber' else: arg_overwrite = 'overwrite'and
from . import arg_overwrite hdulist.writeto(filename, **{arg_overwrite : overwrite})While I certainly wouldn't want to get stuck with a bad API in the name of backwards compatibility, we (Gemini) also find this kind of change a bit disruptive for only cosmetic reasons (more or less)... 😉.
Reacted by Mihai CaraI like @jehturner solution for not relying on
__version__"analysis".
Astropy 1.3 spews deprecation warnings when using
clobbernow. I'm happy to shift fromclobber->overwritebut I have codes that need to maintain compatibility with astropy 1.0+, and I don't want these to throw deprecation warnings everywhere. Is it possible to backport theoverwritechange so that all versions of astropy 1.* support both (with deprecation warnings)? (this is really a policy question regarding minor version changes; I'm sure it's technically possible)