DEPR: Deprecate convert_float

[ahawryluk]

• closes CLN: consider deprecating convert_floats from read_excel #41127
• tests added / passed
• Ensure all linting tests pass, see here for how to run them
• whatsnew entry

I added tm.assert_produces_warning for every existing test that uses the convert_float argument, and a git grep only found one reference to convert_float in doc/. Is anything else required for a deprecation? IIUC, the actual feature would get removed in a pandas 2.0, correct?

[pep8speaks]

Hello @ahawryluk! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-05-25 16:38:24 UTC

[ahawryluk]

I'm failing some checks due the warnings stacklevel. I think stacklevel=5 would be correct if the user calls pd.read_excel, but then it would be wrong for pd.ExcelFile.parse. What would you recommend?

[jreback]

cc @rhshadrach @WillAyd

[rhshadrach]

lgtm, pending the stacklevel issue. I think this can be handled similarly to how the deprecation of xlrd was done.

pandas/pandas/io/excel/_base.py

Lines 1183 to 1198 in f33480d

	caller = inspect.stack()[1]
	if (
	caller.filename.endswith(
	os.path.join("pandas", "io", "excel", "_base.py")
	)
	and caller.function == "read_excel"
	):
	stacklevel = 4
	else:
	stacklevel = 2
	warnings.warn(
	f"Your version of xlrd is {xlrd_version}. In xlrd >= 2.0, "
	f"only the xls format is supported. Install "
	f"openpyxl instead.",
	FutureWarning,
	stacklevel=stacklevel,

IIUC, the actual feature would get removed in a pandas 2.0, correct?

Correct.

[rhshadrach]

lgtm, @jreback you good here?

[jreback]

this is slightly magical. can we instead have a function that shows the deprecation and just call it where appropriate so can set the stacklevel w/o this?

[rhshadrach]

I don't see how this would get around needing logic to set the stacklevel. Maybe you're asking to move the logic to the new function?

[ahawryluk]

@jreback @rhshadrach I've consolidated both stack inspections from _base.py into a single function. It seems like an improvement to me, but let me know if you see other adjustments I should make.

[rhshadrach]

After rereading @jreback's comment, it now makes sense to me. The request is to move the deprecation itself to a function (not the determination of stacklevel), and call this further up in the call stack where we don't need to have any logic to determine the stacklevel. If this is doable, I agree it's a better approach.

[rhshadrach]

I think this would work: move the warnings.warn to a function, call in both read_excel and ExcelFile.parse when appropriate, but in read_excel after emitting the warning change the value of convert_float. Does that make sense?

[jreback]

you might be able to use this: from pandas.util._exceptions import find_stack_level

[ahawryluk]

Oh nice. I looked at pandas.util._exceptions.find_stack_level when I made my last edit, but at the time it seemed specific to "astype" et al. so I tried making something similar. The edits from #41560 two days ago look great. I'll give it a try right away. (I couldn't see a clean way without stack inspection to raise a single warning from both read_excel and ExcelFile.parse. The latter isn't really advertised in the documentation, but it should still raise the correct warnings.)

[simonjayhawkins]

@ahawryluk can you merge master to resolve conflicts

[ahawryluk]

@jreback @rhshadrach I've updated the warnings in pandas/io/excel/_base.py to use the recently improved find_stack_level from pandas.utils._exceptions. One of the test suites failed, but I don't think it's related to excel routines. One of the error messages was "TypeError: Cannot cast array data from dtype('O') to dtype('<U') according to the rule 'unsafe'", and all the others were "FutureWarning: Promotion of numbers and bools to strings is deprecated."

[rhshadrach]

very nice - lgtm

[jreback]

thanks @ahawryluk very nice

[ahawryluk]

Thanks everyone for your help