search

burnash / gspread

Google Sheets Python API
https://docs.gspread.org
MIT License
7.06k stars 945 forks source link

fix: fix type annotation for default_blank #1505

Closed hiro-o918 closed 1 month ago

hiro-o918 commented 1 month ago

fix type annotation

lavigne958 commented 1 month ago

Hi thank you for contributing, I'm sorry but I don't understand the issue behind your changes.

The input default_blank is here to set a value in case of blank cells, when a user wants a specific value. This is typed string so gspread can safely work knowing that all input values are strings.

Could you please explain your changes ?

hiro-o918 commented 1 month ago

@lavigne958 Sorry for slow responding. I found default_black passed into numericise finally. This seems that the value can accept Any value.

For my case, I want to pass pd.NA as a default value for empty data which is not str type.

lavigne958 commented 1 month ago

@lavigne958 Sorry for slow responding. I found default_black passed into numericise finally. This seems that the value can accept Any value.

For my case, I want to pass pd.NA as a default value for empty data which is not str type.

I see, this is a different issue then, you request the ability to use a different type and you know the util functions numericise will work. though the upper method get_all_records will block you from using it.

Then I agree, we need to double check it does not break anything but if this works fine, I'm fine with it.

@alifeee any objection ?

alifeee commented 1 month ago

this sounds good to me

only thing to remember is that multiple functions use default_blank so they should all be updated

not sure of the need for a test since it's just typing.

hiro-o918 commented 1 month ago

@lavigne958 @alifeee Thank you for your reviews. I found there is no additional type annotations str for default_blank please double check just in case

alifeee commented 1 month ago

I think you are correct! Then this is good with me :)