Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Minor type issues in Squeeze #16448

Merged
merged 4 commits into from
Oct 13, 2019
Merged

Conversation

junrushao
Copy link
Member

@junrushao junrushao commented Oct 11, 2019

@zheng-da

This issue caused an incorrect type signature in our auto-generated C++ wrapper in CPP package, link.

With this bug exists, the cpp-package would generate the first argument of signature const std::vector<Symbol> &, but the correct one should be const Symbol &.

Update: as Haibin and Da mentions, this is also the root of the following bug:

mx.np.squeeze(a, axis=1)  # works
mx.np.squeeze(a, 1)       # fails

Copy link
Member

@eric-haibin-lin eric-haibin-lin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a test case where the original code fails? (e.g. nd.squeeze(ones, 1))

@junrushao
Copy link
Member Author

I see...I didn't realize that it fixes an issue that @zheng-da and @eric-haibin-lin mentioned before...I will add a testcase...

@eric-haibin-lin eric-haibin-lin merged commit 1e8cc90 into apache:master Oct 13, 2019
@eric-haibin-lin
Copy link
Member

@junrushao1994 you're the man!

aaronmarkham pushed a commit to aaronmarkham/incubator-mxnet that referenced this pull request Oct 16, 2019
* minor

* test

* Retrigger

* Retrigger
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants