search

pmorissette / ffn

ffn - a financial function library for Python
pmorissette.github.io/ffn
MIT License
2.03k stars 300 forks source link

Replace `applymap` with `map` #226

Closed GuyPago closed 5 months ago

GuyPago commented 5 months ago

Previously, map was used for Series class while applymap was used for the DataFrame class. Now Pandas has deprecated applymap and suggest using map for DataFrame as well.

image

timkpaine commented 5 months ago

This doesn't work on pandas<2.

GuyPago commented 5 months ago

Good comment. I guess requiring >=2.0 from >=0.19 is a pretty major demand, isn't it? Even though the code itself supports 2.2+.

timkpaine commented 5 months ago

Correct, we're not going to require >=2 so the best bet would be to check if pandas > 2/2.1 like we do here and swap the behavior you want to use.

GuyPago commented 5 months ago

Considering the fact that applymap is deprecated rather than removed, it feels like an overkill. If you beg the differ, I'll integrate the change you proposed. Waiting for your response.

timkpaine commented 5 months ago

I made, and then reverted, this same change in https://github.com/pmorissette/ffn/pull/224. So I wasn't really keen to do the work just to avoid a deprecation warning.