-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Conversation
Thanks for fixing this. could there be other errors from the tutorial if we fix the install? |
Is there a way to trigger the test of the tutorial on the CI before merging and waiting for the nightly test? |
Thanks for the fix! @ptrendx AFAIK the tutorials take a long time to run and since the customer impact from a broken tutorial is much lesser, they are not part of the CI. |
@roywei I don't expect any other errors - I was running this tutorial before submitting it ;-). I did not know about the configuration of the tutorial CI, sorry for that! |
@ptrendx It's ok, reproduce on CI could be troublesome. As long as notebooks are running fine locally and dependencies are installed in CI docker, it should be fine. It's documented here.
|
@Chancebair @ptrendx I have tested it and unfortunately it failed again. It seems the tutorial test is treating warning as failures.
|
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.
we need to test this before merging, otherwise nightly will fail again
We can either ignore the warnings or implement the suggestion of providing the type for args. @ptrendx would this be trivial enough? |
@mxnet-label-bot add [Gluon, Installation, pr-awaiting-testing] |
@Chancebair I made a PR to your branch with a fix. |
@Chancebair could you rebase? CI should be passing now. Thanks |
e59e4ee
to
6720553
Compare
Description
To fix the failure here: #15028
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
Comments