-
Notifications
You must be signed in to change notification settings - Fork 170
refactor: Add CompliantDataFrame.from_dict
#2304
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
No typing issues and slightly faster route to the same call
No preference on the name - but I find this a lot easier to read
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 @dangotbanned ! I might have spotted one typo - aside that it looks 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.
This diff 🎻🎻
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.
Ok on a serious not, do you have any resource to learn this the right way?
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.
Ok on a serious not, do you have any resource to learn this the right way?
Haha I guess I'll take that as a compliment 😄
But I'm not sure I know what you mean @FBruzzesi - could you explain?
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.
The easiness with which you are able to introduce and combine typevar's and classes to make all the types coherent - I want to learn that 💡
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.
@FBruzzesi ah that 😂
Well I suppose a lot of it is just putting in the time battling it out with mypy & pyright until things stick.
The most exhaustive resource would be the typing spec.
I use that whenever I come across a message/concept I'm not familiar with.
If you use VSCode, I strongly recommend enabling the inlay hints settings for Pylance.
When in doubt, you can just omit/delete an annotation and see what pyright has inferred it to be.
That usually helps nudge me towards solving an issue - if that doesn't match what I expected.
I'll try to have a think on it tomorrow if there's anything else I can share
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.
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.
that's nice! just enabled it, thanks!
Co-authored-by: Francesco Bruzzesi <[email protected]>
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.
|
@MarcoGorelli @FBruzzesi thanks both for the review! |


What type of PR is this? (check all applicable)
Related issues
*(Namespace|DataFrame).from_numpy#2283.from_iterable()innew_series#2302Checklist
If you have comments or can explain your changes, please do so below