Keep default values in df_kwargs when calling load_data()#270
Conversation
Codecov Report
@@ Coverage Diff @@
## master #270 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 19 19
Lines 568 571 +3
=========================================
+ Hits 568 571 +3
|
pwildenhain
left a comment
There was a problem hiding this comment.
PR looks great 🤩 just one consideration I have for the eav record type.
If the user asks for that then they'll be given one row per column per record, instead of the usual one row per record. So in that case we don't want to define an index_col
| ], | ||
| format_type: Literal["json", "csv", "xml", "df"], | ||
| df_kwargs: Optional[Dict[str, Any]] = None, | ||
| record_type: Literal["flat", "eav"] = "flat", |
There was a problem hiding this comment.
Is there a reason you've removed this argument? I would still like to be able to have the user return a data frame for the eav record type option
There was a problem hiding this comment.
Not sure anymore. I guess, I did not see, that content = 'report when calling _load_data() from export_records(). I modified the code to leave the df_kwargs unchanged for the eav record type option.
| df_kwargs = {"index_col": "original_field_name"} | ||
| df_kwargs = {} | ||
|
|
||
| if "index_col" not in df_kwargs.keys() and record_type != "eav": |
There was a problem hiding this comment.
This check leaves the df_kwargs unchanged when the record type is eav. Let me know if that sounds reasonable. Else, I'll change it back to the previous behavior of defining an empty dict for df_kwargs.
pwildenhain
left a comment
There was a problem hiding this comment.
🙇🏻♂️ Thanks for contributing this improvement 🎉
This PR implements issue #268.
Previously, the default values in the df_kwargs were overridden, even if no value was specified for them.
Now, the default values remain unchanged (e.g., for
index_colswhen usingexport_records(df_kwargs{'low_memory': False})).Additionally, three test cases are implemented to verify the above described behavior, as well as the existing one.
Besides, the command to install poetry in the contribution guidelines is update.