refactor: Add CompliantDataFrame.from_dict#2304
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
FBruzzesi
left a comment
FBruzzesi
left a comment
There was a problem hiding this comment.
Thanks @dangotbanned ! I might have spotted one typo - aside that it looks great 👌
There was a problem hiding this comment.
Ok on a serious not, do you have any resource to learn this the right way?
There was a problem hiding this comment.
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.
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.
@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.
Inlay hints
Now that I think about it, it must seem strange that I rarely annotate variables 😄
There was a problem hiding this comment.
that's nice! just enabled it, thanks!
Co-authored-by: Francesco Bruzzesi <42817048+FBruzzesi@users.noreply.github.com>
|
@MarcoGorelli @FBruzzesi thanks both for the review! |
3 participants
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