CLN remove exception catching in string_to_dts

[MarcoGorelli]

The except? -1 can be removed

pandas/pandas/_libs/tslibs/np_datetime.pyx

Lines 274 to 283 in e2598a1

	cdef int string_to_dts(
	str val,
	npy_datetimestruct* dts,
	NPY_DATETIMEUNIT* out_bestunit,
	int* out_local,
	int* out_tzoffset,
	bint want_exc,
	format: str | None=None,
	bint exact=True,
	) except? -1:

as string_to_dts is only ever called with want_exc=False

This function would need amending to raise ValueError if string_to_dts failed

pandas/pandas/_libs/tslib.pyx

Lines 79 to 100 in 502919e

	def _test_parse_iso8601(ts: str):
	"""
	TESTING ONLY: Parse string into Timestamp using iso8601 parser. Used
	only for testing, actual construction uses `convert_str_to_tsobject`
	"""
	cdef:
	_TSObject obj
	int out_local = 0, out_tzoffset = 0
	NPY_DATETIMEUNIT out_bestunit
	obj = _TSObject()
	string_to_dts(ts, &obj.dts, &out_bestunit, &out_local, &out_tzoffset, True)
	obj.value = npy_datetimestruct_to_datetime(NPY_FR_ns, &obj.dts)
	check_dts_bounds(&obj.dts)
	if out_local == 1:
	obj.tzinfo = timezone(timedelta(minutes=out_tzoffset))
	obj.value = tz_localize_to_utc_single(obj.value, obj.tzinfo)
	return Timestamp(obj.value, tz=obj.tzinfo)
	else:
	return Timestamp(obj.value)

[ramvikrams]

Just wanted to clear a confusion we have to add the raise the ValueError in the else block

[MarcoGorelli]

Hey - I'm not 100% what would be best yet, I'll get back to you on this

[ShashwatAgrawal20]

@MarcoGorelli any updates ?

[ShashwatAgrawal20]

take

[MarcoGorelli]

I'll give some details later this week (though if you want to start investigating, feel free to go ahead!)

[ShashwatAgrawal20]

I'll give some details later this week (though if you want to start investigating, feel free to go ahead!)

The string_to_dts function returns a special error value '(-1)' as if an error occurs, but the _test_parse_iso8601 isn't checking for it, we can simply check the return value of string_to_dts if it's -1 we can raise a ValueError

Any thoughts?

[ShashwatAgrawal20]

@MarcoGorelli any updates?

[MarcoGorelli]

do you want to see if it's possible to always have want_exc as True, and to use a try-except instead of checking the value of string_to_dts_failed? This might give a more informative error message

[ShashwatAgrawal20]

do you want to see if it's possible to always have want_exc as True, and to use a try-except instead of checking the value of string_to_dts_failed? This might give a more informative error message

Like this in '_test_parse_iso8601'? & setting the want_exc to 'True' in 'string_to_dts'

obj = _TSObject()

try:
    string_to_dts(ts, &obj.dts, &out_bestunit, &out_local, &out_tzoffset)
except Exception as e:
    raise ValueError(f"Error parsing timestamp: {ts}. Reason: {e}. Make sure the timestamp is in the correct format.")

[MarcoGorelli]

there shouldn't be any need to try-except in _test_parse_iso8601 (it already has want_exc as True), only in other places which call string_to_dts but with want_exc as False, such as

pandas/pandas/_libs/tslibs/strptime.pyx

Lines 311 to 329 in 9a5702c

	string_to_dts_failed = string_to_dts(
	val, &dts, &out_bestunit, &out_local,
	&out_tzoffset, False, fmt, exact
	)
	if not string_to_dts_failed:
	# No error reported by string_to_dts, pick back up
	# where we left off
	value = npy_datetimestruct_to_datetime(NPY_FR_ns, &dts)
	if out_local == 1:
	# Store the out_tzoffset in seconds
	# since we store the total_seconds of
	# dateutil.tz.tzoffset objects
	tz = timezone(timedelta(minutes=out_tzoffset))
	result_timezone[i] = tz
	out_local = 0
	out_tzoffset = 0
	iresult[i] = value
	check_dts_bounds(&dts)
	continue

[ShashwatAgrawal20]

there shouldn't be any need to try-except in _test_parse_iso8601 (it already has want_exc as True), only in other places which call string_to_dts but with want_exc as False, such as

pandas/pandas/_libs/tslibs/strptime.pyx

Lines 311 to 329 in 9a5702c

	string_to_dts_failed = string_to_dts(
	val, &dts, &out_bestunit, &out_local,
	&out_tzoffset, False, fmt, exact
	)
	if not string_to_dts_failed:
	# No error reported by string_to_dts, pick back up
	# where we left off
	value = npy_datetimestruct_to_datetime(NPY_FR_ns, &dts)
	if out_local == 1:
	# Store the out_tzoffset in seconds
	# since we store the total_seconds of
	# dateutil.tz.tzoffset objects
	tz = timezone(timedelta(minutes=out_tzoffset))
	result_timezone[i] = tz
	out_local = 0
	out_tzoffset = 0
	iresult[i] = value
	check_dts_bounds(&dts)
	continue

Ohh! A wrap-up call?

try:
    string_to_dts(val, &dts, &out_bestunit, &out_local, &out_tzoffset, False, fmt, exact)
    value = npy_datetimestruct_to_datetime(NPY_FR_ns, &dts)
    if out_local == 1:
        tz = timezone(timedelta(minutes=out_tzoffset))
        result_timezone[i] = tz
        out_local = 0
        out_tzoffset = 0
    iresult[i] = value
    check_dts_bounds(&dts)
    continue
except Exception as e:
    raise ValueError(f"Error parsing timestamp: {val}. Make sure the timestamp is in the correct format. Error message: {e}")

[MarcoGorelli]

it still needs to continue to the code below it if it failed, so more like

try:
    string_to_dts(...)
except:
    pass
else:
    value = npy_datetimestruct_to_datetime(NPY_FR_ns, &dts)
    if out_local == 1:
        tz = timezone(timedelta(minutes=out_tzoffset))
        result_timezone[i] = tz
        out_local = 0
        out_tzoffset = 0
    iresult[i] = value
    check_dts_bounds(&dts)
    continue    

[ShashwatAgrawal20]

Got it, should be something like this if I am not wrong.

try:
    string_to_dts(val, &dts, &out_bestunit, &out_local, &out_tzoffset, False, fmt, exact)
except ValueError as e:
    #pass
    raise ValueError(f"Error parsing timestamp: {val}. Make sure the timestamp is in the correct format. Error message: {e}")
else:
    value = npy_datetimestruct_to_datetime(NPY_FR_ns, &dts)
    if out_local == 1:
        tz = timezone(timedelta(minutes=out_tzoffset))
        result_timezone[i] = tz
        out_local = 0
        out_tzoffset = 0
    iresult[i] = value
    check_dts_bounds(&dts)
    continue