RF/MNT: Remove some rust #915
Conversation
The package requires numpy >= 1.13, np.flip was added in 1.12 Add deprecation Remove dedicated test
Complies with python_requires >= 3.6 Was produced with flynt
Remove also related tests
Codecov Report
@@ Coverage Diff @@
## master #915 +/- ##
==========================================
+ Coverage 91.73% 92.14% +0.40%
==========================================
Files 97 97
Lines 12359 12268 -91
Branches 2177 2162 -15
==========================================
- Hits 11338 11304 -34
+ Misses 684 627 -57
Partials 337 337
Continue to review full report at Codecov.
|
|
Hi, thanks a lot for these suggestions!
Sounds good to me. Generally we go next major + 1, to be extra conservative about removing things.
I'd considered doing this for 3.1 in #854, but the This wasn't highly principled, but I imagined there's a reasonable likelihood somebody might have missed 3.0.0 and 3.0.1 and moved straight to 3.1.0. Also, in the run-up to 4.0 we'll have to effect the deprecations scheduled then, so it might be cleanest to make a big sweep and clear out the dead-as-of-3.0 code then, too. Anyway, happy to discuss making a coherent policy. It might be that 3.2, which will be ~6 months after 3.0, is far enough along.
👍
Not at all! I appreciate the initiative. One thing that might be worth considering is splitting out the efforts into separate, focused PRs. That way, a trivial change that hits many files, like using double quotes on docstrings, can be reviewed separately from a more consequential one in a single file, such as the |
|
Thanks for the reply! I agree that separate PRs are better, let's do this. Regarding the deprecations - deferring is ok, but I do not get the reason behind a special error for over-deprecated objects. It means that these deprecations will stay forever in the package. Don't we want to remove them at some point? |
It gives people who ignored warnings an informative error. We usually try to direct them to the replacement.
Yes. I figured a major version (~1 year; see #734) would be a reasonable interval to expect even pretty slow-moving ecosystems to run into it. |
|
Sounds good. |
|
Do you mean changes like the following? @property
- @deprecate_with_version('ArrayProxy.header deprecated', '2.2', '3.0')
+ @deprecate_with_version('ArrayProxy.header deprecated', '2.2', '4.0')
def header(self):
return self._headerIf so, no. That would revert the exception (which breaks things) to a warning (which can be ignored). The idea of not removing immediately is to break things, but with an informative error. In the run-up to 4.0, we can clear out all the code. If not, could you clarify what you mean? |
|
With that one, because of the missing comma, the docstring was actually rendered:
Instead of raising a warning between 1.3 and then an error at 3.0, it only started raising a warning at 3.0. Because we did attempt to deprecate it at 1.3, I'm a bit inclined to fast-track it and target 4.0 rather than 5.0 for raising an |
flip_axisin orientations module ->np.flip+ deprecation==> RF: Use np.flip instead of flip_axis #916
==> Not yet, use special Errors (out of the scope of this PR).
==> MNT: Fix configparser + python 2 deprecations #917
''' ... '''to""" ... """==> STY: Use triple double-quoted string in docstrings (PEP-257) #918
%/.formatto f-strings (there's still some work in this point)==> STY: Old formatting to f-strings (PEP-498) #919
Might be a bit rude because there was no prior discussion... but we can discuss here :-)