Patch pandas.core.dtypes.inference.is_dict_like#141
Patch pandas.core.dtypes.inference.is_dict_like#141WillDaSilva wants to merge 2 commits intoKxSystems:masterfrom
pandas.core.dtypes.inference.is_dict_like#141Conversation
Pandas thinks all non-atom K objects are dicts, which leads to issues like KxSystems#133. This commit provides a hacky solution to this problem by overwriting the `__code__` attribute of the function Pandas uses for checking if an object is dict-like. Patching the function through safer means (e.g. with `unittest.mock`) won't work well because the function is imported all over Pandas, and we would have to patch all of them. Fixes KxSystems#133
|
This pull request introduces 1 alert when merging 79786e8 into 8c31f19 - view on LGTM.com new alerts:
|
| html/ | ||
|
|
||
| .venv*/ | ||
| venv*/ |
There was a problem hiding this comment.
Would you rather a separate PR just for this change? The .gitignore should exclude Python virtual environments in some capacity.
There was a problem hiding this comment.
I don't believe venvs belong in code repository. For example, I keep my venvs in the .virtualenvs directory. Adding all possible locations will grow .gitignore dramatically and this is not necessary.
There was a problem hiding this comment.
Per-project virtual environments are a widespread practice. You'll find most large Python projects have a few lines in their .gitignore to exclude them. Besides, we don't need to add "all possible locations" - no need to let perfect be the enemy of good. This addition takes care of all of the most common ones in just two lines.
|
@cmccarthy1 I'm leaving this to @abalkin to review. |
4 participants
Pandas thinks all non-atom K objects are dicts, which leads to issues like #133. This PR provides a hacky solution to this problem by overwriting the
__code__attribute of the function Pandas uses for checking if an object is dict-like. Patching the function through safer means (e.g. withunittest.mock) won't work well because the function is imported all over Pandas, and we would have to patch all of them.Fixes #133