-
Notifications
You must be signed in to change notification settings - Fork 906
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
improve %load_node
support full node syntax
#3633
improve %load_node
support full node syntax
#3633
Conversation
Signed-off-by: Nok Lam Chan <nok.lam.chan@quantumblack.com>
Signed-off-by: Nok Lam Chan <nok.lam.chan@quantumblack.com>
Signed-off-by: Nok Lam Chan <nok.lam.chan@quantumblack.com>
Signed-off-by: Nok Lam Chan <nok.lam.chan@quantumblack.com>
Signed-off-by: Nok Lam Chan <nok.lam.chan@quantumblack.com>
Signed-off-by: Nok Lam Chan <nok.lam.chan@quantumblack.com>
Signed-off-by: Nok Lam Chan <nok.lam.chan@quantumblack.com>
…ect instead of generate string for cells directly Signed-off-by: Nok Lam Chan <nok.lam.chan@quantumblack.com>
Signed-off-by: Nok Lam Chan <nok.lam.chan@quantumblack.com>
Signed-off-by: Nok Lam Chan <nok.lam.chan@quantumblack.com>
Signed-off-by: Nok Lam Chan <nok.lam.chan@quantumblack.com>
Signed-off-by: Nok Lam Chan <nok.lam.chan@quantumblack.com>
… element Signed-off-by: Nok Lam Chan <nok.lam.chan@quantumblack.com>
Signed-off-by: Nok Lam Chan <nok.lam.chan@quantumblack.com>
@lrcouto It's ready for test, I have refactor the logic and manual tested and unit tests are all passed. |
Signed-off-by: lrcouto <laurarccouto@gmail.com>
Signed-off-by: lrcouto <laurarccouto@gmail.com>
Signed-off-by: lrcouto <laurarccouto@gmail.com>
@astrojuanlu here's the issue I mentioned today at standup. This function at |
Was the issue Sphinx is trying to render that method in docs? Can we somehow suppress this, tbh there is not much value to document the class in the API docs, we can even convert it to a private class |
Signed-off-by: lrcouto <laurarccouto@gmail.com>
…ithub.com:kedro-org/kedro into noklam/improve-load-node-support-full-node-syntax
Signed-off-by: lrcouto <laurarccouto@gmail.com>
I think it is. I've been trying to find a way to either suppress warnings from external libraries, or suppress this specific warning, but I don't think I can do that without ignoring the whole file. I'll try to see if converting it to a private class helps. |
Signed-off-by: lrcouto <laurarccouto@gmail.com>
Converted NodeBoundArguments to private and that solved this specific instance of the problem. It'd still be a good idea to find a more permanent solution for this issue though. |
@lrcouto Great! I think we have seen this issue in the past, I don't really remember how we solved it, maybe @AhdraMeraliQB or @astrojuanlu knows more? The solution is to not use some of the |
Tagging @DmitriiDeriabinQB and @ankatiyar for review |
The other @DimedS :) |
The way I've usually solved it is by committing the template generated by sphinx.ext.autosummary during the docs building process and removing the classes or properties I don't want to have documented. Sphinx will then abstain from overwriting the file. Good that you solved the problem for now 👍🏽 I don't think this was expected to be public API anyway? |
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.
Tested this, and it works!
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.
Description
Fix #3629
%load_node
support full Kedro Node syntax, with better *args, **kwargs handling #3629Development notes
2 main changes:
NodeBoundArguments
, which inherit frominspect.BoundArguments
. . This allow the helper function to return a proper data structure for testing. The structure will also be useful for generating all the necessary text so our tests can focus on testing on the data structure, not the exact string.BoundArguments
, we can easily generate the necessary function signature according to the node function, the only thing we need to do is taking care of the "*args" to handle a specific edge case.Developer Certificate of Origin
We need all contributions to comply with the Developer Certificate of Origin (DCO). All commits must be signed off by including a
Signed-off-by
line in the commit message. See our wiki for guidance.If your PR is blocked due to unsigned commits, then you must follow the instructions under "Rebase the branch" on the GitHub Checks page for your PR. This will retroactively add the sign-off to all unsigned commits and allow the DCO check to pass.
Checklist
RELEASE.md
file