-
Notifications
You must be signed in to change notification settings - Fork 10
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
Implemented upper and lower triangle methods in NumRuby #45
Conversation
@iamrajiv Add some tests. Also, indentation needs to be fixed. |
m = m.stype(nm_dtype, copy=False) | ||
return m | ||
end | ||
|
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.
I think that missing and end
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.
I think lines 72
to 76
are having extra indentation.
lib/nmatrix/numruby.rb
Outdated
def tril(m, k=0) | ||
m = Array(m) | ||
mask = tri(*m.shape[-2:], k=k, nm_dtype=nm_bool) | ||
return where(mask, m, zeros(1, m.nm_dtype)) |
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.
I think this return
isn't necessary 🤔
lib/nmatrix/numruby.rb
Outdated
def triu(m, k=0) | ||
m = Array(m) | ||
mask = tri(*m.shape[-2:], k=k-1, nm_dtype=nm_bool) | ||
return where(mask, zeros(1, m.nm_dtype), m) |
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.
I think this return
isn't necessary 🤔
@iamrajiv pls have a look at the suggestions. |
lib/nmatrix/numruby.rb
Outdated
@@ -62,4 +62,27 @@ def self.hstack(objs) | |||
result | |||
end | |||
|
|||
def self.tri(N, M=None, k=0, nm_dtype=float) | |||
if M is None: |
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.
I suppose that None
is an object that you created. But I think that you can use nil
instead of it. Also I think that isn't necessary use the :
to the end of sentence. WDYT? 🤔
@iamrajiv tests are failing, there seems to be some syntax errors. Make sure to run tests first before committing. |
to resolve the test, I think you could change the Capital Letter in the parameters by lower case, because ruby take the capital letter like a constant. WDYT? |
Fixes: #29