BUG: in Timestamp.replace when replacing tzinfo around DST changes

[jreback]

replaces #17356
closes #15683

[codecov]

Codecov Report

Merging #17507 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #17507      +/-   ##
==========================================
- Coverage   91.18%   91.16%   -0.02%     
==========================================
  Files         163      163              
  Lines       49543    49543              
==========================================
- Hits        45177    45168       -9     
- Misses       4366     4375       +9

Flag	Coverage Δ
#multiple	88.95% <ø> (ø)	⬆️
#single	40.21% <ø> (-0.07%)	⬇️

Impacted Files	Coverage Δ
pandas/io/gbq.py	25% <0%> (-58.34%)	⬇️
pandas/core/frame.py	97.77% <0%> (-0.1%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 83436af...1782097. Read the comment docs.

[codecov]

Codecov Report

Merging #17507 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #17507      +/-   ##
==========================================
- Coverage   91.19%   91.18%   -0.02%     
==========================================
  Files         163      163              
  Lines       49626    49626              
==========================================
- Hits        45258    45249       -9     
- Misses       4368     4377       +9

Flag	Coverage Δ
#multiple	88.96% <ø> (ø)	⬆️
#single	40.19% <ø> (-0.07%)	⬇️

Impacted Files	Coverage Δ
pandas/io/gbq.py	25% <0%> (-58.34%)	⬇️
pandas/core/frame.py	97.77% <0%> (-0.1%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 21a3800...66d94a8. Read the comment docs.

[sscherfke]

Really? You don’t really help with my PR for half a year and than you just replace it?

I hope I will never have to contribute to this project again.

[gfyoung]

@sscherfke : I haven't followed this PR (and its history) too closely. However, based on what I have read, let me see if I can unpack this:

@jreback put up this PR 9 days after the last comments regarding your PR (#17356). Generally, this is quite early to do a replacement PR. That being said, I suspect the decision to do this was driven by the fact that there was lot more work to address on the performance and code quality sides.

From your previous comments, it sounded like you did not have much free time or energy left to devote to doing that (understandable, no one's life is purely just this repository). Thus, it might have been easier logistically for us to take this under our wing and off your plate so that we could take it to the finish line.

[gfyoung]

I hope I will never have to contribute to this project again.

I'm sorry you feel this way. Contributing to open source can sometimes be difficult, especially when the interests of the contributors and the maintainers don't fully align (I've sat on both sides of this conversation). It happens, but that doesn't mean we don't appreciate the effort you've taken to help make pandas better. Mind you that we're taking our own free time out of our busy lives to do this, so we have to use it wisely. The fact that several of us have chosen to take the time to review your PR says something.

I should add though that you are not forced to use the official installations for your development. You can apply these patches (in their current form) to your own installed version of pandas, and you don't have to worry about it getting merged into the official installation for the time being.

[jreback]

your commit is still here - and you will get the credit for it

[sscherfke]

I’ve been contributing to and maintaining my own OSS projects since many years now—and it wasn’t always easy. But this issue has been the most nerve-racking and exhausting one.

I reported the issue more than half a year ago (I guess) and when I finally convinced you that it was actually a bug I was told to make a PR. I asked for help because the Pandas source is not very easy to work with if you have never worked with and contributed to Pandas before. I received verly-little to no help except for comments like “not good enought”, “it’s not fast enough” (implying that fast is better than correct), and “please move your change log entries to the new history file” (every couple of weeks when there was a new release).

Now that I made a new, clean PR that finally received a “looks quite good“ you just replace it without discussion which felt very rude to me. I don’t care about the credit, I care about nice treatment of contributors (and especially first-time contributors) and I care about correct software. It feels like I have just wasted too much time on this project.

(We already apply this patch in our own Conda recipe for pandas.)

[jreback]

@sscherfke your comments are quite disengenous. We have 2000 open issues and 100 PR's. Sure your PR is certainly helpful as it does fix a bug. But you never really responded to the perf concerns or put benchmarking in place.

I made a new PR, but your commit is the same whether it exists in the existing code or new. Not really sure what you are bothered by.

implying that fast is better than correct), and “please move your change log entries to the new history file” (every couple of weeks when there was a new release).

This is a very odd comment. Sure correct is always better. But it is very hard to assess the performance impact of a very critical code path without performance testing.

You picked a really technical issue. I am glad that it is solved, though I still have perf concerns for the changes.

I haven't decided to actually merge this yet; it still needs more work.

[jreback]

merging. I think the impact will be limited to only .replace on Timestamps, so perf hit is ok.