-
Notifications
You must be signed in to change notification settings - Fork 6.8k
[Clojure] follow up pr 14531 to fix tests #14565
Conversation
@Chouffe would you mind taking a look and reviewing this when you get a chance? |
@gigasquid I will take a look shortly when I get a chance. Thanks for finishing the fixes ^^ |
@@ -35,3 +35,4 @@ | |||
ndarray-or-double-or-float | |||
#{"org.apache.mxnet.MX_PRIMITIVES$MX_PRIMITIVE_TYPE" | |||
"org.apache.mxnet.NDArray"}))))) | |||
|
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.
#nitpick do we need this extra line?
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.
It's in the generator - not needed but we can tackle that in another PR touching that area.
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.
🎉
$ lein test
...
...
Ran 230 tests containing 805 assertions.
0 failures, 0 errors.
We should open an issue for silencing logs in tests. I will add an issue
(m/forward data-batch)) | ||
;; FIXME | ||
#_(is (= [(first l-shape) num-class] | ||
(m/forward data-batch-2)) |
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.
awesome catch!
@mxnet-label-bot add [Clojure, test] |
Thanks for reviewing @Chouffe ! |
Description
This is a follow up PR to #14531 which fixed some Clojure tests that were not testing what they thought they were.
This PR resolves the FIXME disabled tests in there