Separate replace & shift function #276
Conversation
according to PR Request from issue arrow-py#220
|
Excellent, I'll review this and get it merged. |
|
Looks solid. Let me do some testing locally before pushing out this breaking change. |
|
Oops, I miss something, the docs, |
|
💯 : this is a huge usability gain ! |
|
Would there be a way to make replace call shift so that it doesn't break backward compatibility? |
|
@pypingou That shouldn't be a problem; however, I don't know if that's the best option. Let's hear what @andrewelkins says. I could do another PR if needed although I prefer the way this is. |
|
@sipp11 how would it not be a problem? All the calls to |
|
@pypingou Sorry I might reply you too short since it should have been "That shouldn't be a problem to add deprecation warning." For the breaking change, I'm fine either way since I've fixed them all. Code is much easier to read :) |
|
It's easier to read and I think the change is great but breaking backward compatibility from a release to another is bad. I do think we should announce the change with a deprecation warning for a release (ideally two), then we can clean the code and get ride of |
|
That makes sense. Can we add the deprecation warning? |
Replace w/shift function will be removed in next release hopefully
|
Sorry that I couldn't do it sooner, but if you still need it, I restore replace functionality (to shift date) and add deprecation warning if using it. Hopefully I'll get another PR done too. I use this library too much and I hate to just pip from my fork. |
…d sipp11:shift Conflicts: arrow/arrow.py tests/arrow_tests.py
|
I've resolved the merge conflicts in #349 |
|
Thanks @sipp11 for the original PR and @Syeberman I'll check out the new PR and close this one. |
5 participants
According to PR Request from the issue #220, I also think it's a nice idea.
Basically ripping relativedelta part out of replace() to make a new function, shift() & refactoring all related tests.