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

DOC: Change datatypes of A matrices in linprog #9362

Merged
merged 4 commits into from
Oct 15, 2018

Conversation

Kai-Striega
Copy link
Member

closes #9323

  • Change datatype of A_ub from 1D array to 2D array
  • Change dataype of A_eq from array_like to 2D array
  • Add full description of tableau T in simplex method
  • Shorten descriptions of A_ub, A_eq and A

* Change datatype of ``A_ub`` from ``1D array`` to ``2D array``
* Change dataype of ``A_eq`` from ``array_like`` to ``2D array``
* Add full description of tableau ``T`` in simplex method
* Shorten descriptions of ``A_ub``, ``A_eq`` and ``A``
2D array which, when matrix-multiplied by ``x``, gives the values of
the upper-bound inequality constraints at ``x``.
A_ub : 2D array, optional
2D array such that ``A_ub`` @ ``x`` gives the values of the upper-bound
Copy link
Member

Choose a reason for hiding this comment

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

I think @ sign doesn't render well in the docs between two code markups. But it is my subjective view.

Copy link
Member

Choose a reason for hiding this comment

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

The whole expression A @ x is code.

Copy link
Member

Choose a reason for hiding this comment

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

I meant these ones

image

Copy link
Member Author

Choose a reason for hiding this comment

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

@ilayn I thought the convention is to mark variables as variable_name? Looking at the picture I agree it looks odd. If you think it's worth rewriting I can do it tomorrow.

Copy link
Member

@pv pv Oct 7, 2018

Choose a reason for hiding this comment

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

I guess the relevant guidelines are:
https://numpydoc.readthedocs.io/en/latest/format.html
http://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html

The double-backtick quotes I think for the most part are used for inline code samples, which A_ub @ x in principle is.
Also note that the tableau table code should probably be put in a multiline literal block.

Copy link
Member Author

Choose a reason for hiding this comment

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

From the numpy guidelines:

Enclose variables in single backticks.

So, technically, this should be: A @ x, right? To me A @ x still looks better/readable and worth going back and changing.

Copy link
Member

Choose a reason for hiding this comment

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

Yes since you are referring to a math expression rather than the individual variables involved, I guess A @ x would be clearer. But again, just a suggestion, I don't have a strong opinion.

@ilayn
Copy link
Member

ilayn commented Oct 7, 2018

Since we are at it, the return parameter in

A `scipy.optimize.OptimizeResult` consisting of the following fields:

doesn't have an argument and hence marks the whole sentence as the output type argument. Maybe something like

Returns
--------
opt_res : OptimizeResult
    A :class:`scipy.optimize.OptimizeResult` object with the following fields 
    ....

with a clickable link can be more informative.

@rgommers rgommers added Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org scipy.optimize labels Oct 10, 2018
@Kai-Striega
Copy link
Member Author

@ilayn do you think we can merge this?

Copy link
Member

@ilayn ilayn left a comment

Choose a reason for hiding this comment

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

I see that the _ub variables have A_ub @ x in their docstrings but _eq variables A_eq @ x. Is that a copy/paste slip or they have different meanings? I think I didn't get that point.

But if we are happy with those, it is good to go. Please go ahead and merge. They look good to me.

the equality constraints at ``x``.
b_eq : array_like, optional
A_eq : 2D, optional
2D array such that ``A_eq`` @ ``x`` gives the values of the equality
Copy link
Member

Choose a reason for hiding this comment

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

code formatting of @

2D array such that ``A_ub @ x`` gives the values of the upper-bound
inequality constraints at ``x``.
A_eq : 2D array, optional
2D array such that ``A_eq`` @ ``x`` gives the values of the equality
Copy link
Member

Choose a reason for hiding this comment

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

code formatting of @

2D array such that ``A_ub @ x`` gives the values of the upper-bound
inequality constraints at ``x``.
A_eq : 2D array, optional
2D array such that ``A_eq`` @ ``x`` gives the values of the equality
Copy link
Member

Choose a reason for hiding this comment

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

code formatting of @

the equality constraints at ``x``.
b_eq : array_like, optional
A_eq : 2D array, optional
2D array such that ``A_eq`` @ ``x`` gives the values of the equality
Copy link
Member

Choose a reason for hiding this comment

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

code formatting of @

the equality constraints at ``x``.
b_eq : 1D array
A_eq : 2D array, optional
2D array such that ``A_eq`` @ ``x`` gives the values of the equality
Copy link
Member

Choose a reason for hiding this comment

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

code formatting of @

the equality constraints at ``x``.
b_eq : array_like
A_eq : 2D array, optional
2D array such that ``A_eq`` @ ``x`` gives the values of the equality
Copy link
Member

Choose a reason for hiding this comment

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

code formatting of @

the equality constraints at ``x``.
b_eq : array_like, optional
A_eq : 2D array, optional
2D array such that ``A_eq`` @ ``x`` gives the values of the equality
Copy link
Member

Choose a reason for hiding this comment

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

code formatting of @

b_eq : 1D array
constraint (row) in ``A_ub``.
A_eq : 2D array, optional
2D array such that ``A_eq`` @ ``x`` gives the values of the equality
Copy link
Member

Choose a reason for hiding this comment

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

code formatting of @

1D array of values representing the upper-bound of each inequality
constraint (row) in ``A_ub``.
A_eq : 2D array, optional
2D array such that ``A_eq`` @ ``x`` gives the values of the equality
Copy link
Member

Choose a reason for hiding this comment

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

code formatting of @

1D array of values representing the upper-bound of each inequality
constraint (row) in ``A_ub``.
A_eq : 2D array, optional
2D array such that ``A_eq`` @ ``x`` gives the values of the equality
Copy link
Member

Choose a reason for hiding this comment

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

code formatting of @

@Kai-Striega
Copy link
Member Author

Kai-Striega commented Oct 14, 2018

@ilayn Yes, that was a copy paste slip, I've changed it now and will merge once the tests pass.

@ilayn ilayn added this to the 1.2.0 milestone Oct 14, 2018
@ilayn
Copy link
Member

ilayn commented Oct 14, 2018

The Travis failure is not related and is due to #9190 that uses numpy.moveaxis which is newer than our least supported NumPy version 1.8.2. I'll fix that but I don't understand why it didn't fail on that PR.

@ilayn
Copy link
Member

ilayn commented Oct 14, 2018

I've just restarted the failing case to make sure it wasn't a hiccup of some sort.

@Kai-Striega Kai-Striega merged commit 8047001 into scipy:master Oct 15, 2018
@Kai-Striega Kai-Striega deleted the doc_9323 branch October 15, 2018 05:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org scipy.optimize
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Data types of linprog inputs
5 participants