-
Notifications
You must be signed in to change notification settings - Fork 835
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
fixes foldering of the gpt2 minio notebook #4197
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Hi @saeid93. Thanks for your PR. I'm waiting for a SeldonIO or todo member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the jenkins-x/lighthouse repository. |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: axsaucedo The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/unapprove |
/hold |
@saeid93 it seems there are some lint issues, we use formatters for the notebooks as well so running those with the same versions should fix it |
@axsaucedo I couldn't find the lint issue, could you please have a look and let me know where should I fix? |
For sure, you can see the issue on the logs for the The way to fix it is to run the nbqa with the same version as the one we use to ensure all the notebooks are reformatted accordingly. You can find this command in the makefile, ie To check
To modify
|
thanks for the explanation, hopefully, it'll work this time. |
NIce one @saeid93 - merging |
What this PR does / why we need it:
As @RafalSkolasinski truly commented on my last commit there was still problems in the previous merged notebook. I made some changes to the foldering and can confirm this version works fine on my local. Please review.
Which issue(s) this PR fixes:
Fixes #4080
Special notes for your reviewer: