-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
Improve error message for DataFrame.from_dict when wrong orient is provided #47451
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
I don't think any tests are needed for this, since it really is such a small fix. Could be wrong though :/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small comments, might also break tests if error message was checked
Damn yall really test everything lol. Ok I will fix the failing test, at least now I have a good idea of which test to fix. |
I think these failures are unrelated |
Ah. Ok I will write new tests then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made requested changes.
Was unable to test locally, due to failing test elsewhere, so I will leave it to CI to run this test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice fix, thanks for working on this. lgtm, just added couple of suggestions.
@@ -189,3 +184,16 @@ def test_frame_dict_constructor_empty_series(self): | |||
# it works! | |||
DataFrame({"foo": s1, "bar": s2, "baz": s3}) | |||
DataFrame.from_dict({"foo": s1, "baz": s3, "bar": s2}) | |||
|
|||
def test_from_dict_scalars_requires_index(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: I moved this function down here, so that all functions that test exception behaviours are grouped together. Not sure if that complies to the guidelines, but it certainly seems better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @KianShah, lgtm
thx @KianShah |
…ovided (pandas-dev#47451) * Fixed error message * Update v1.4.3.rst * Update v1.4.3.rst * Added test case * Added test * Fix rst file * Fix black issues * Fixed error msg and corresponding test * improved changelog * Also fix python docstring for :func:to_dict * Fix failing test
doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.