GH1173 Experiment with fuller typing

[loicdiridollou]

• Closes type kwargs in DataFrame.query according to DataFrame.eval #1173
• [] Tests added: Please use assert_type() to assert the type of any return value

@MarcoGorelli I took a try at your idea of typehinting the whole set of arguments in pd.DataFrame.query but stumbling upon the issue when the user passes a dictionary (which is still an allowed behavior).
Wondering if there is something I am missing here, please let me know!

[Dr-Irv]

@MarcoGorelli I took a try at your idea of typehinting the whole set of arguments in pd.DataFrame.query but stumbling upon the issue when the user passes a dictionary (which is still an allowed behavior).
Wondering if there is something I am missing here, please let me know!

You'd still need to have the **kwargs as an overload. If you want to allow the other arguments, you can have that (although I don't think we should do that because it's not documented), but you'd still need the **kwargs overload

[loicdiridollou]

I was able to make some progress here, let me know if this is what you envisioned and I will clean it up.

[MarcoGorelli]

Hey - yeah, this is what I was thinking - I don't really understand why **kwargs would be needed at all, given that the accepted arguments by eval are limited https://pandas.pydata.org/docs/reference/api/pandas.eval.html#pandas.eval , although based on

If you want to allow the other arguments, you can have that (although I don't think we should do that because it's not documented)

it looks like Irv disagrees and that what I've suggested may be against the pandas-stubs philosophy - which is fair enough

[Dr-Irv]

Hey - yeah, this is what I was thinking - I don't really understand why **kwargs would be needed at all, given that the accepted arguments by eval are limited https://pandas.pydata.org/docs/reference/api/pandas.eval.html#pandas.eval , although based on

If you want to allow the other arguments, you can have that (although I don't think we should do that because it's not documented)

it looks like Irv disagrees and that what I've suggested may be against the pandas-stubs philosophy - which is fair enough

So I looked more carefully at the pandas docs, which do say that the accepted **kwargs are from eval(). Not the greatest on the docs front, and that could be improved. Maybe one of you could create an issue there??

So I'll look more carefully at this PR now with that in mind.

[Dr-Irv]

Not clear why you need this overload when the previous overload covers it?

[loicdiridollou]

This test would raise a mypy/pyright error without the other overload.

pandas-stubs/tests/test_frame.py

Lines 532 to 536 in d19ac89

	kwargs = {"parser": "pandas", "engine": "numexpr"}
	check(
	assert_type(df.query("col1 > col2", inplace=False, **kwargs), pd.DataFrame),
	pd.DataFrame,
	)

[Dr-Irv]

same comment - not sure why we need this overload when the previous one covers it.

[loicdiridollou]

The goal was to still allow for someone passing kwargs as a dictionary instead of the individual arguments since it is the way documented in the docs like df.query("col1 > col2", **kwargs) that you would pass from another function.

[loicdiridollou]

The idea is that someone may put a wrapper like:

def my_own_query(df, expr, **kwargs):
    return df.query(expr, **kwargs)

If you drop the second overload this will raise an error.

[Dr-Irv]

duplicate tests

[loicdiridollou]

Fixed

[loicdiridollou]

I will raise an issue on the pandas side if it is not simpler to have all the arguments directly in the function. It is a good point since it should not be too hard to maintain.