-
Notifications
You must be signed in to change notification settings - Fork 71
Condense doc for is_square into single docstring #2186
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
746ec2b
ddd7300
707b4cc
ea4d8b4
c049a50
0d87449
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -152,6 +152,7 @@ end | |
| # | ||
| ################################################################################ | ||
|
|
||
| # See generic documentation in NCRings.jl | ||
| is_square(a::MatRingElem) = true | ||
|
||
|
|
||
| ############################################################################### | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -176,6 +176,17 @@ function is_nilpotent(a::T) where {T <: NCRingElement} | |||||
| end | ||||||
|
|
||||||
|
|
||||||
| @doc raw""" | ||||||
| is_square(a::T) where {T <: NCRingElement} | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Of course this is a very minor complaint. But at least to me these function signatures are confusing, I always start wondering if there is a reason to write it with the type parameter. And I don't see a reason to write it this way in a docstring. |
||||||
| is_square(M::MatElem) | ||||||
| is_square(M::MatRingElem) | ||||||
|
|
||||||
| There are two distinct situations: | ||||||
| (1) if the argument is matrix then check whether the matrix **shape** is square | ||||||
| (2) if the argument is not a matrix then check whether the **value** is a square (in the same ring) | ||||||
| """ | ||||||
| function is_square end | ||||||
|
Comment on lines
179
to
185
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMHO this really should be two docstrings:
Both then can be included in different parts of the manual (one in the ring interface section, one in matrix interface section).
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ping @JohnAAbbott
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apparently already done. I'll un-draft-ify the PR. |
||||||
|
|
||||||
| ############################################################################### | ||||||
| # | ||||||
| # Characteristic | ||||||
|
|
||||||
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.
While I understand the intention, I note that we do not have such comments in front of the many other methods that implement ring interfaces, so it seems odd to have the comment added to just
is_square. I'd rather not add this everywhere at all.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.
OK - removed. Will push shortly.