dyvenia / viadot

A simple library to move data between places.
https://viadot.docs.dyvenia.com
MIT License
25 stars 40 forks source link

Refactor `SAPBW` source #1066

Open angelika233 opened 1 day ago

angelika233 commented 1 day ago

https://github.com/dyvenia/viadot/pull/1053#discussion_r1778744268

angelika233 commented 1 day ago

remove mapping_dict from to_df()

trymzet commented 1 day ago

Seems like this _apply_mapping is actually doing 2 things: first it filters a dataframe, and then renames the columns. Eg. if we have a filter + mapping dict like this: {"my_col": "my_new_col_name"} and a df with 2 columns, "my_col" and "my_other_col", after applying this function, we'd get a df with 1 column, "my_new_col_name".

So I guess this function is confusing for 3 reasons:

So it should probably be split into 2 functions or just native dataframe functions used (filtering a df and renaming columns are done with a single line of code in core pandas, so it's likely wrappers are not needed).