Skip to content

DOC: add NumPy import in scipy.optimize examples #16832

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

Merged
merged 3 commits into from
Aug 16, 2022
Merged

Conversation

ak04p
Copy link
Contributor

@ak04p ak04p commented Aug 12, 2022

Reference issue

#16759

What does this implement/fix?

Added import numpy as np to docstring examples for scipy.optimize

Additional information

@ak04p ak04p requested a review from andyfaff as a code owner August 12, 2022 06:14
@AtsushiSakai AtsushiSakai added Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org scipy.optimize labels Aug 13, 2022
@@ -274,6 +274,7 @@ def savemat(file_name, mdict,
Examples
--------
>>> from scipy.io import savemat
>>> import numpy as np
Copy link
Member

@AtsushiSakai AtsushiSakai Aug 13, 2022

Choose a reason for hiding this comment

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

Should this line be at the top of the code block based on this comment?
I think other code changes should obey the rule also.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll make the necessary changes then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've made changes to scipy.optimize . However, changes to scipy.io have already been merged. So should I change it or leave it as is?

Copy link
Member

@AtsushiSakai AtsushiSakai Aug 14, 2022

Choose a reason for hiding this comment

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

I realized that this change is not for the "optimize" module now. If the main branch already has this change, could you please rebase this PR branch on the main branch? Or if not, could you please remove changes in the io module from this PR? I think this PR only focuses on changes for the optimize module.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the feedback, I've removed changes in the io module from this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've made the changes according to your review.

@@ -638,6 +639,7 @@ def least_squares(
estimate it by finite differences and provide the sparsity structure of
Jacobian to significantly speed up this process.

>>> import numpy as np
Copy link
Member

@AtsushiSakai AtsushiSakai Aug 14, 2022

Choose a reason for hiding this comment

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

I think this import statement is not needed because of the rule: "Add them a maximum of once per "Example" section."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll take note of this.

@@ -744,6 +746,7 @@ def least_squares(
We wrap it into a function of real variables that returns real residuals
by simply handling the real and imaginary parts as independent variables:

>>> import numpy as np
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 this import statement is not needed because of the rule: "Add them a maximum of once per "Example" section."

@@ -711,6 +712,7 @@ def curve_fit(f, xdata, ydata, p0=None, sigma=None, absolute_sigma=False,

Examples
--------
>>> import numpy as np
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
>>> import numpy as np
>>> import numpy as np
>>> import matplotlib.pyplot as plt

@@ -1015,6 +1020,7 @@ def check_grad(func, grad, x0, *args, epsilon=_epsilon,
... return x[0]**2 - 0.5 * x[1]**3
>>> def grad(x):
... return [2 * x[0], -1.5 * x[1]**2]
>>> import numpy as np
Copy link
Member

Choose a reason for hiding this comment

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

This should move to the top of the Example section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll fix this now.

@@ -2742,6 +2750,7 @@ def bracket(func, xa=0.0, xb=1.0, args=(), grow_limit=110.0, maxiter=1000):
This function can find a downward convex region of a function:

>>> import matplotlib.pyplot as plt
>>> import numpy as np
Copy link
Member

Choose a reason for hiding this comment

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

"import numpy as np" should be first than "import matplotlib.pyplot as plt".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll take note of this.

@@ -136,6 +136,7 @@ def root(fun, x0, args=(), method='hybr', jac=None, tol=None, callback=None,
... return [x[0] + 0.5 * (x[0] - x[1])**3 - 1.0,
... 0.5 * (x[1] - x[0])**3 + x[1]]

>>> import numpy as np
Copy link
Member

Choose a reason for hiding this comment

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

This should be at the top of the Example section.

@@ -163,6 +164,7 @@ def root(fun, x0, args=(), method='hybr', jac=None, tol=None, callback=None,

The solution can be found using the ``method='krylov'`` solver:

>>> import numpy as np
Copy link
Member

Choose a reason for hiding this comment

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

This should be the top of the example section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking since this was a separate example, I should add import numpy as np. I'll remove it since it has already been added to the top of the examples section.

@@ -210,6 +210,7 @@ class of similar problems can be solved together.

Examples
--------
>>> import numpy as np
>>> from scipy import optimize
>>> import matplotlib.pyplot as plt
Copy link
Member

Choose a reason for hiding this comment

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

This plt import should be next to the np import

@tupui tupui added this to the 1.10.0 milestone Aug 16, 2022
@tupui
Copy link
Member

tupui commented Aug 16, 2022

LGTM now thank you @ak04p and @AtsushiSakai for the review.

@tupui tupui merged commit b3277e0 into scipy:main Aug 16, 2022
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.

3 participants