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

DEP: Deprecate private namespaces in scipy.optimize #14966

Merged
merged 3 commits into from
Nov 6, 2021

Conversation

czgdp1807
Copy link
Member

Reference issue

gh-14360

What does this implement/fix?

Additional information

cc: @rgommers

@Smit-create Smit-create added the maintenance Items related to regular maintenance tasks label Nov 2, 2021
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

The second commit needs a couple of changes to setup.py - there are still moduleTNC.so and minpack2.so left. Could you give those an underscore as well?

One other minor issue I spotted.

scipy/optimize/lbfgsb.py Outdated Show resolved Hide resolved
@rgommers
Copy link
Member

rgommers commented Nov 2, 2021

Also, CI is still broken.

@czgdp1807
Copy link
Member Author

there are still moduleTNC.so and minpack2.so left. Could you give those an underscore as well?

Ah! okay. No worries. I will do that.

@czgdp1807 czgdp1807 force-pushed the optimize branch 2 times, most recently from 2cbca6e to 9200c3d Compare November 2, 2021 18:28
@czgdp1807
Copy link
Member Author

I am fixing mypy errors. Will push after verifying locally.

@rgommers
Copy link
Member

rgommers commented Nov 3, 2021

The doc build is still broken, trying to use optimize.optimize

@czgdp1807
Copy link
Member Author

I get the following warning in docs,

docstring of _odrpack.odr:16: WARNING: py:obj reference target not found: ODR
docstring of _odrpack.odr:28: WARNING: py:obj reference target not found: ODR
/Users/czgdp1807/scipy_project/scipy/build/testenv/lib/python3.9/site-packages/scipy/optimize/_root.py:docstring of scipy.optimize._root.root:86: WARNING: py:obj reference target not found: nonlin
/Users/czgdp1807/scipy_project/scipy/build/testenv/lib/python3.9/site-packages/scipy/odr/__init__.py:docstring of scipy.odr:29:<autosummary>:1: WARNING: py:obj reference target not found: scipy.odr.ODR
/Users/czgdp1807/scipy_project/scipy/build/testenv/lib/python3.9/site-packages/scipy/optimize/__init__.py:docstring of scipy.optimize:396: WARNING: py:mod reference target not found: scipy.optimize.nonlin
release/0.11.0-notes.rst:85: WARNING: py:obj reference target not found: nonlin
release/1.6.0-notes.rst:47: WARNING: py:obj reference target not found: scipy.odr.ODR

And the following message,

build finished with problems, 14 warnings.
make: *** [html-scipyorg] Error 1
Traceback (most recent call last):
  File "/Users/czgdp1807/scipy_project/scipy/runtests.py", line 566, in <module>
    main(argv=sys.argv[1:])
  File "/Users/czgdp1807/scipy_project/scipy/runtests.py", line 215, in main
    subprocess.run(cmd, check=True)
  File "/Users/czgdp1807/mambaforge/envs/scipy-dev/lib/python3.9/subprocess.py", line 528, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['make', '-Cdoc', 'PYTHON="/Users/czgdp1807/mambaforge/envs/scipy-dev/bin/python"', 'html-scipyorg', 'latex', 'SPHINXOPTS="-j2"']' returned non-zero exit status 2.

Are they okay?

@czgdp1807
Copy link
Member Author

I think other test failures are unrelated?

@rgommers
Copy link
Member

rgommers commented Nov 3, 2021

I'll have a look, the docs warnings are slightly odd - unclear to me why the ODR ones are showing up now for example.

@czgdp1807
Copy link
Member Author

I think in release notes we use module names before this deprecation. Hence, now nonlin doesn't exist because it's converted to _nonlin. Should we update release notes as well?

@rgommers
Copy link
Member

rgommers commented Nov 3, 2021

I think in release notes we use module names before this deprecation. Hence, now nonlin doesn't exist because it's converted to _nonlin. Should we update release notes as well?

yes we should, just to avoid doc build warnings

@rgommers
Copy link
Member

rgommers commented Nov 3, 2021

Hmm, I fixed most things, but nonlin is a little annoying. While there is nothing in optimize.nonlin that is not also present in optimize, it had its own submodule docs for some reason: http://scipy.github.io/devdocs/reference/optimize.nonlin.html#module-scipy.optimize.nonlin.

I'd prefer to get rid of that, moving to _nonlin and merging the docs content with the main optimize docs. But I want to flag that and make sure we're okay with that. The alternative is to add optimize.nonlin to the list of public API modules in http://scipy.github.io/devdocs/reference/index.html#api-definition and revert some of the changes in this PR (and keep the duplicate exposing of functions in the two namespaces). Anyone have any objections to getting rid of the nonlin namespace @scipy/scipy-core-team?

@larsoner
Copy link
Member

larsoner commented Nov 4, 2021

Anyone have any objections to getting rid of the nonlin namespace @scipy/scipy-core-team?

None here, sounds like a good plan to me

@rgommers
Copy link
Member

rgommers commented Nov 6, 2021

Only thumbs up for not making nonlin public. Also, I realized after writing this that all functions in it are legacy, root(..., method='xxx') is recommended instead.

CI is green except for the expected linting complaints, so in it goes. Thanks @czgdp1807, all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Items related to regular maintenance tasks scipy.optimize
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants