QT tests are failing - not this PR

100% agree that pandas datetimes + bar should be tested.

OTOH, I'm not 100% convinced its matplotlib that should be responsible for those tests if the pandas datetime unit converters are registered, which they are in this case. See#11664 for where I propose we explictlyunregister pandas datetime unit conversion so then we can test our own converters (which work with basic pandas objects without the need for their converters). This was prompted byimport pandas no longer registering their converters by default. They backed down on that change, but may re-impliment it at some point.

Not to say pandas' own converters shouldn't work - but it seems that they should be responsible for reporting breakages to us (like this one). However, I'm open to the idea we should fully test pandas if that is how it should be done.

For this particular case, I'm pretty ignorant about how the user got to the point wherebar worked at all (#13186), so I'm hesitant to add to the tests. But if someone knoweledgeable of pandas wanted to add or propose a test, I'm more than happy for that to go in...

Have we reported the breakage to pandas? If that is the case, we may want to simply add a test and a comment that this should be fixed in pandas version XXX, and remove the work around and and the test when times come.
Does that seem reasonable ?

Hmm, looking at it again, I guess pandas thinks its a ValueError becauseself.freq is None.

In any case, not convinced this needs a test from our end, but the change should go in to make theexcept clause here as liberal as possible. In fact I'd argue its should be a bare except - if addingconvert(x+dx) fails for any reason give us the old behaviour of just callingconvert(dx)

... ah apparentlyexcept Exception is the way to do this cleanly without a dangerous bare exception. Hopefully that is deemed OK by folks...

The current solution catches all exceptions, which we should avoid. It means will accidentally catch errors and exception we don't want to catch. I don't think the proposed solution is reasonable (sorry!)

(I need to run, but I'll review this more carefully in the next day or so)

@NelleV I'm happy to go back to explicit exception types, butexcept Exceptions is the pep-8 way to make exception as liberal as possible:

A bare except: clause will catch SystemExit and KeyboardInterrupt exceptions, making it harder to interrupt a program with Control-C, and can disguise other problems. If you want to catch all exceptions that signal program errors, use except Exception: (bare except is equivalent to except BaseException:).

so I thought this would be OK.

https://www.python.org/dev/peps/pep-0008/

@jklymak But making catching exception as liberal as possible is not good practice. It means the code "works", in cases where it should probably fail. It actually happened to me several time with Matplotlib that the plot would show and be wrong due to broad error catching such as this one.

I'll dismiss my review, as I am about to board a plane and will be offline for the next 24h, but I think it is better to be as narrow and explicit as possible when catching errors.

Tightened exception

@dopplershift I can't make the try-except block shorter because the error is sometimes in the procedure that gets the first element.

Just to be clear - the idea is "try some stuff to get this to work. If that fails just do what we did before this PR". So having this try/except block makes things a bit better at no cost (other than code complexity).

OTOH I don't feel passionately about this solution. If someone has a better solution forbar that would be great.

@jklymak That's fine--but then for a block that big, I agree with@NelleV that we should be as specific as possible in the exceptions caught. IMHOexcept Exception: is only ok when there's one specific line that you're catching errors from.

@dopplershift sure, but is the new version OK? It has three explicit errors.

That PR was missing a test. I'll create a PR to fix this.

Oops. Thanks.