Specifying `fill_value` in `pd.Series.eq()` with `'string'` dtype

[geoffrey-eisenbarth]

Right now I see the pandas docs show that fill_value should be None or float value, but if the Series has dtype 'string', then using a string value works as expected (although mypy obviously balks).

I know this is kind of a pandas issue too; I tried navigating the pandas issue tracker for something mentioning it and found this: pandas-dev/pandas#15533. I got a little lost trying to parse everything in the issue, so I'm a little unsure of where this stands; would a pandas doc PR be sufficient for now? Should pandas-stubs support passing a string to fill_value?

I'm happy to put together any necessary PRs here or for docs in the pandas repo if it's agreed upon that this is the correct approach.

[loicdiridollou]

Hey @geoffrey-eisenbarth, thanks for the report.
I think the spirit of the stubs is to enhance what the docs are offering and not go against what pandas documents so I am not in favor of such changes (especially when there is no clear direction from pandas, in case the PR was there and ready to merge I would be okay with going a little ahead but that is not the case here).
What you can do is comment on the issue on the pandas side (seems like it is a super old one with little activity recently to check on the progress.
The problem we may face with providing undocumented behaviors is that today it is accepted but it may suddenly break so a little dangerous.
Maybe @Dr-Irv and @cmp0xff can argue against my view but I would prefer to have pandas go ahead and confirm that such behavior is allowed before we take any decision to make changes here.
Before we settle on a way to proceed you will have to type: ignore on your side. Hope that helps!

[cmp0xff]

I agree with @loicdiridollou , that it could start with fixing the docs issue in pandas first, and then we could adapt pandas-stubs.

[Dr-Irv]

I agree with @loicdiridollou , that it could start with fixing the docs issue in pandas first, and then we could adapt pandas-stubs.

And I agree as well.

[geoffrey-eisenbarth]

Sounds good, I'll get it sorted over there and then circle back to this. Thanks!

[loicdiridollou]

@geoffrey-eisenbarth any chance you could reference the pandas issue here, it may take some time to resolve on pandas side but good to be able to keep an eye on progress. Thanks!

[geoffrey-eisenbarth]

@loicdiridollou I've mentioned it here pandas-dev/pandas#15533 (comment) but perhaps I should open a brand new pandas issue for this?

[loicdiridollou]

I was just looking at which issue you commented on the pandas side so we can check progress there and when it is resolved on pandas we can see it in this conversation so we know it’s good to start working on it, normally it shows up as a mention but I guess GitHub changed the formatting

[krishgarg344]

Just flagging that pandas-dev/pandas#66371 updates the fill_value docs to include str. If it merges, happy to work on the stubs fix

[krishgarg344]

Also am curious that pandas-dev/pandas#66371 updates the type hint to Hashable rather than just adding str. Is Hashable the right choice for the stubs fix too, or should it be more specific?

[loicdiridollou]

I am not super familiar with this part of the code but I believe it should accept any elements of S1 (in that any elements that Series accepts, fill_value should accept since this is an element compatible with the Series).
Actually if you try passing a Timestamp or a list, this will fail, Timestamp is a hashable, bool however works. So I guess you should add str and bool at least.

[krishgarg344]

Thanks! So float | str | bool | None would be more accurate than Hashable for the stubs fix?

[loicdiridollou]

Best is probably to try out all the potential elements that would be supported by a Series and see which one are accepted by the eq method, I already found that bool is accepted so they may be more.

[krishgarg344]

Tested with dtype='string' only str, None, and np.nan work. bool, float, int all raise TypeError. So float | str | None seems like the right type for the stubs fix. This also contradicts your earlier observation that bool works, Could be dependant on dtype?

[loicdiridollou]

Hmm sorry my bad I had a Series[float], so you may need an overload since bool is accepted in certain cases. Let's wait for the pandas PR to merge and then we can tackle this one.