8000 MNT Remove sklearn.utils.fixes.loguniform by MaxwellLZH · Pull Request #23730 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

MNT Remove sklearn.utils.fixes.loguniform #23730

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

MNT Remove sklearn.utils.fixes.loguniform #23730

wants to merge 3 commits into from
+2 −29

Conversation

@MaxwellLZH
Copy link
Contributor
@MaxwellLZH MaxwellLZH commented Jun 22, 2022

Reference Issues/PRs

Fixes #23699.

What does this implement/fix? Explain your changes.

Replaces utils.fixes.loguniform with scipy.stats.loguniform, and removed corresponding test case.

@ogrisel
Copy link
Member
ogrisel commented Jun 22, 2022

We need to bump up the dependency of scipy to 1.5 or 1.6 in the _min_dependencies.py file first (and regenerate the CI config with build_tools/update_environments_and_lock_files.py).

Currently, the min-dependencies doc build fails with:

Traceback (most recent call last):
  File "/home/runner/mambaforge/envs/testenv/lib/python3.8/site-packages/sphinx/cmd/build.py", line 280, in build_main
    app.build(args.force_all, filenames)
  File "/home/runner/mambaforge/envs/testenv/lib/python3.8/site-packages/sphinx/application.py", line 387, in build
    self.events.emit('build-finished', None)
  File "/home/runner/mambaforge/envs/testenv/lib/python3.8/site-packages/sphinx/events.py", line 101, in emit
    results.append(listener.handler(self.app, *args))
  File "/home/runner/mambaforge/envs/testenv/lib/python3.8/site-packages/sphinx_gallery/gen_gallery.py", line 640, in summarize_failing_examples
    raise ExtensionError(
sphinx.errors.ExtensionError: Here is a summary of the problems encountered when running the examples
Unexpected failing examples:
/home/runner/work/scikit-learn/scikit-learn/examples/gaussian_process/plot_compare_gpr_krr.py failed leaving traceback:
Traceback (most recent call last):
  File "/home/runner/mambaforge/envs/testenv/lib/python3.8/site-packages/sphinx_gallery/gen_gallery.py", line 159, in call_memory
    return 0., func()
  File "/home/runner/mambaforge/envs/testenv/lib/python3.8/site-packages/sphinx_gallery/gen_rst.py", line 466, in __call__
    exec(self.code, self.fake_main.__dict__)
  File "/home/runner/work/scikit-learn/scikit-learn/examples/gaussian_process/plot_compare_gpr_krr.py", line 180, in <module>
    from scipy.stats import loguniform
ImportError: cannot import name 'loguniform' from 'scipy.stats' (/home/runner/mambaforge/envs/testenv/lib/python3.8/site-packages/scipy/stats/__init__.py)

@lesteve
Copy link
Member
lesteve commented Jun 23, 2022

As I tried to indicate in the related issue, we tend to do bump minimum requirements and update sklearn.utils.fixes close to the release which will roughly happen in November (we try to do a release every 6 months) ...

Basically I created the issue more as a reminder to ourselves than anything else ...

@MaxwellLZH
Copy link
Contributor Author
MaxwellLZH commented Jun 23, 2022 •
edited
Loading

As I tried to indicate in the related issue, we tend to do bump minimum requirements and update sklearn.utils.fixes close to the release which will roughly happen in November (we try to do a release every 6 months) ...

Basically I created the issue more as a reminder to ourselves than anything else ...

Maybe we can leave this PR as it is for now and revisit when close to the next release :)

@lesteve
Copy link
Member
lesteve commented Jun 23, 2022

Maybe we can leave this PR as it is for now and revisit when close to the next release :)

Sounds good to me!

@ogrisel ogrisel added this to the 1.2 mil 8000 estone Jun 24, 2022
@lesteve
Copy link
Member
lesteve commented Jul 6, 2022

Note for ourselves in a few months, there are still instances of sklearn.utils.fixes.loguniform left in this PR branch.

It should be removed from the doc completely. Either we remove loguniform from sklearn/utils/fixes.py or we deprecate it and remove it in two releases. Not sure the status of sklearn.utils.fixes.py with regards to the two releases deprecation cycle policy.

❯ git grep -P 'fixes.+loguniform' | grep -v whats_new
doc/modules/grid_search.rst::class:`~sklearn.utils.fixes.loguniform`. This is a continuous version of
doc/modules/grid_search.rst:  from sklearn.utils.fixes import loguniform
examples/model_selection/plot_randomized_search.py:from sklearn.utils.fixes import loguniform
sklearn/utils/fixes.py:    >>> from sklearn.utils.fixes import loguniform

@thomasjpfan
Copy link
Member
thomasjpfan commented Jul 6, 2022 •
edited
Loading

Not sure the status of sklearn.utils.fixes.py with regards to the two releases deprecation cycle policy.

Historically, we usually still deprecate over two versions for anything in utils.

Linking issue: I proposed a faster deprecation cycle for utils #15801

@thomasjpfan
Copy link
Member
thomasjpfan commented Jul 6, 2022

Looking at another PR: #22894 In that case we removed sparse_lsqr without deprecation.

@betatim
Copy link
Member
betatim commented Sep 28, 2022

@MaxwellLZH do you want to reboot this PR now that other deprecation related PRs are getting merged for the upcoming release?

@betatim
Copy link
Member
betatim commented Oct 14, 2022

I'll make a new PR for this

@jeremiedbb
Copy link
Member
jeremiedbb commented Oct 14, 2022

Moving it to the 1.3 milestone since we did not bump scipy min version in the end.

@jeremiedbb jeremiedbb modified the milestones: 1.2, 1.3 Oct 14, 2022
@betatim betatim mentioned this pull request Oct 14, 2022
Merged
@cmarmo cmarmo added the Superseded PR has been replace by a newer PR label Nov 21, 2022
@jeremiedbb
Copy link
Member
jeremiedbb commented Mar 24, 2023

done in #24665

@jeremiedbb jeremiedbb closed this Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

module:utils Superseded PR has been replace by a newer PR

Projects

None yet

Milestone

1.3

Development

Successfully merging this pull request may close these issues.

Remove sklearn.utils.fixes.loguniform for 1.3 release

7 participants

@MaxwellLZH @ogrisel @lesteve @thomasjpfan @betatim @jeremiedbb @cmarmo
0