Skip to content
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

Fix issue #13449 related to matrices.py docStrings #13461

Conversation

biswesh456
Copy link

added double backticks to showcase the variable names and function names inside the matrices.py

added double backticks to showcase the variable names and function names inside the matrices.py
@@ -711,7 +711,7 @@ def cross_cancel(a, i, b, j):
return self._new(self.rows, self.cols, mat), tuple(pivot_cols), tuple(swaps)

def echelon_form(self, iszerofunc=_iszero, simplify=False, with_pivots=False):
"""Returns a matrix row-equivalent to `self` that is
"""Returns a matrix row-equivalent to ```self``` that is
Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing you don't want triple tics here. (?)

simplify : Function
A function used to simplify elements when looking for a pivot.
By default SymPy's `simplify`is used.
By default SymPy's ``simplify``is used.
Copy link
Member

Choose a reason for hiding this comment

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

space after tic (before "is")?

@@ -1133,10 +1133,10 @@ def eigenvals(self, error_when_incomplete=True, **flags):
return eigs

def eigenvects(self, error_when_incomplete=True, **flags):
"""Return list of triples (eigenval, multiplicity, basis).
"""Return list of ``triples (eigenval, multiplicity, basis)``.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure the tics should be around this. This could also be phrased, "Return a tuple containing (in order) the eigenvalue, multiplicity and basis.

@@ -1474,8 +1474,8 @@ def pick_vec(small_basis, big_basis):
def left_eigenvects(self, **flags):
"""Returns left eigenvectors and eigenvalues.

This function returns the list of triples (eigenval, multiplicity,
basis) for the left eigenvectors. Options are the same as for
This function returns the list of ``triples (eigenval, multiplicity,
Copy link
Member

Choose a reason for hiding this comment

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

same as above regarding tics and phrasing

@smichr
Copy link
Member

smichr commented Oct 16, 2017

I (almost) never read anything but the ASCII so don't understand the nuance between single and double bac tics, so I"m not sure if all these changes are needed/right. Someone with more back-tic-foo should have a look.

@smichr
Copy link
Member

smichr commented Oct 17, 2017

Duplicates #13459

@biswesh456
Copy link
Author

@smichr Thanks for your input. I have made changes accordingly.

an N x 1 vector whose entries are coefficients of the
polynomial

charpoly(M) = det(t*I - M)
``charpoly(M) = det(t*I - M)``
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't need double backticks here. Rather some .. math .. would be good. See as an example https://github.com/sympy/sympy/blob/master/sympy/concrete/products.py#L31-L33.

@@ -595,7 +595,7 @@ def _normalize_op_args(self, op, col, k, col1, col2, error_str="col"):

def _permute_complexity_right(self, iszerofunc):
"""Permute columns with complicated elements as
far right as they can go. Since the `sympy` row reduction
far right as they can go. Since the ``sympy`` row reduction
Copy link
Contributor

Choose a reason for hiding this comment

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

Change this to SymPy without any backticks, neither single nor double backticks.


D = P^-1 * M * P
``D = P^-1 * M * P``
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't use double backticks here. Use .. math:: ... here as has been described above for one of the changes.

@@ -4286,8 +4286,8 @@ def _normalize_slices(row_slice, col_slice):
return (row_slice, col_slice)

def _coord_to_index(i, j):
"""Return the index in _mat corresponding
to the (i,j) position in the matrix. """
"""Return the index ``in _mat`` corresponding
Copy link
Contributor

Choose a reason for hiding this comment

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

should in be inside the double backticks?

(pivot_offset, pivot_val, assumed_nonzero, newly_determined)
where pivot_offset is the index of the pivot, pivot_val is
the (possibly simplified) value of the pivot, assumed_nonzero
``(pivot_offset, pivot_val, assumed_nonzero, newly_determined)``
Copy link
Contributor

Choose a reason for hiding this comment

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

don't double backtick this.

where pivot_offset is the index of the pivot, pivot_val is
the (possibly simplified) value of the pivot, assumed_nonzero
``(pivot_offset, pivot_val, assumed_nonzero, newly_determined)``
where ``pivot_offset`` is the index of the pivot, ``pivot_val`` is
Copy link
Contributor

Choose a reason for hiding this comment

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

don't double backtick these.

the (possibly simplified) value of the pivot, assumed_nonzero
``(pivot_offset, pivot_val, assumed_nonzero, newly_determined)``
where ``pivot_offset`` is the index of the pivot, ``pivot_val`` is
the (possibly simplified) value of the pivot, ``assumed_nonzero``
Copy link
Contributor

Choose a reason for hiding this comment

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

don't double backtick this.

is True if an assumption that the pivot was non-zero
was made without being proved, and newly_determined are
was made without being proved, and ``newly_determined`` are
Copy link
Contributor

Choose a reason for hiding this comment

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

don't double backtick this.

@gxyd gxyd added PR: author's turn The PR has been reviewed and the author needs to submit more changes. and removed Needs Review labels Oct 23, 2017
@asmeurer
Copy link
Member

asmeurer commented Oct 23, 2017

@smichr the double ticks are required to render as code in Sphinx/RST. A single backtick renders as LaTeX math.

@asmeurer
Copy link
Member

By the way, this makes me think about changing the behavior of single ticks #13519

@divyanshu132
Copy link
Member

Fixed in #15950.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation matrices PR: author's turn The PR has been reviewed and the author needs to submit more changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants