Skip to content

Comments

Use workers instead of alternate parameter names#7302

Merged
stefanv merged 11 commits intoscikit-image:mainfrom
lagru:use-num_workers
Apr 28, 2025
Merged

Use workers instead of alternate parameter names#7302
stefanv merged 11 commits intoscikit-image:mainfrom
lagru:use-num_workers

Conversation

@lagru
Copy link
Member

@lagru lagru commented Jan 19, 2024 •
edited
Loading

Description

Closes #4876. Refer to the closed issue for the original discussion, which settled on workers or num_workers as the consistent name to use. I opted for the latter here, as it seemed more explicit. Current consensus is to follow at least one pre-existing naming convention, in this case the one from SciPy.

Checklist

Release note

Summarize the introduced changes in the code block below in one or a few sentences. The
summary will be included in the next release notes automatically:

Deprecate parameter `num_threads` in `skimage.restoration.rolling_ball`; 
use `workers` instead.

Deprecate parameter `num_workers` in `skimage.restoration.cycle_spin`; 
use `workers` instead.

@lagru
Use "num_workers" instead of alternate versions
a2d12ec 
"num_workers" is inline with what dask uses and also the most common
pattern currently for this kind of parameter.
@lagru lagru added 🔽 Deprecation Involves deprecation 📜 type: API Involves API change(s) 🥾 Path to skimage2 A step towards the new "API 2.0" labels Jan 19, 2024
@stefanv
Copy link
Member

stefanv commented Jan 19, 2024

I will comment on the original issue, but I'm not sure we landed on the correct naming.

@jarrodmillman
Copy link
Contributor

jarrodmillman commented Jan 25, 2024

Maybe this should be coordinated across packages via a SPEC.

@lagru lagru mentioned this pull request Feb 26, 2024
Closed
mkcor
mkcor approved these changes Feb 27, 2024
View reviewed changes
@mkcor mkcor added this to the 0.23 milestone Feb 28, 2024
@lagru lagru marked this pull request as draft March 10, 2024 14:21
@lagru
Copy link
Member Author

lagru commented Mar 10, 2024

Marking this as a draft for now.

@lagru lagru added the ⬆️ Upstream Involves an upstream project label Mar 10, 2024
@jarrodmillman jarrodmillman removed this from the 0.23 milestone Apr 10, 2024
@github-actions
Copy link

github-actions bot commented Oct 8, 2024

Hello scikit-image core devs! There hasn't been any activity on this PR for more than 180 days. I have marked it as "dormant" to make it easy to find.
To our contributors, thank you for your contribution and apologies if this contribution fell through the cracks! Hopefully this ping will help bring some fresh attention to the review. If you need help, you can always reach out on our forum
If you think that this PR is no longer relevant, you may close it, or we may do it at some point (either way, it will be done manually). If you think the PR is valuable but you no longer have the bandwidth to update it, please let us know, so that someone can take it over. 🙏

@github-actions github-actions bot added the 😴 Dormant No recent activity label Oct 8, 2024
@lagru lagru added this to skimage2 Mar 3, 2025
@github-project-automation github-project-automation bot moved this to To Do in skimage2 Mar 3, 2025
@lagru lagru moved this from To Do to Stalled / Waiting in skimage2 Mar 3, 2025
@lagru lagru removed this from skimage2 Mar 3, 2025
mkcor
mkcor reviewed Mar 26, 2025
View reviewed changes
lagru and others added 2 commits March 29, 2025 11:33
@lagru @mkcor
Update skimage/restoration/_rolling_ball.py
bd061a1 
Co-authored-by: Marianne Corvellec <[email protected]>
@lagru
Refactor to workers to align with SciPy's name convention
1d2051d 
After more discussion, the current consensus is to follow at least one
pre-existing convention, in this case the one from SciPy.
@lagru lagru marked this pull request as ready for review March 29, 2025 14:53
@lagru lagru changed the title Use num_workers instead of alternate parameter names Use workers instead of alternate parameter names Mar 29, 2025
@lagru lagru added this to the 0.26 milestone Mar 29, 2025
@lagru lagru requested a review from mkcor March 29, 2025 14:59
@mkcor
Copy link
Member

mkcor commented Mar 29, 2025

Why this CI failure all of a sudden? 🤔
ImportError: PyWavelets is not installed. Please ensure it is installed in order to use this function.

@lagru
Copy link
Member Author

lagru commented Apr 3, 2025

Because I forgot to mark a new test as xfail in case pywavelets isn't available. Should pass now and be ready for review.

mkcor
mkcor reviewed Apr 15, 2025
View reviewed changes
lagru added 2 commits April 21, 2025 18:48
@lagru
Address missing dask warning in test_cycle_spinner_deprecate_num_workers
e23f553 
We need to first, ignore the warning if it is raised, and also make
sure that it has the correct stacklevel. For strange reasons,
pytest.warns includes the warning in `records` even if the category and
message don't match. So we need to ensure that is passes
`assert_stacklevel`.
@lagru
Merge branch 'main' into use-num_workers
ebef2a0
@lagru lagru removed the 😴 Dormant No recent activity label Apr 22, 2025
@lagru lagru added this to skimage2 Apr 22, 2025
@github-project-automation github-project-automation bot moved this to To Do in skimage2 Apr 22, 2025
@lagru lagru moved this from To Do to To Review in skimage2 Apr 22, 2025
@lagru lagru removed this from skimage2 Apr 22, 2025
@stefanv stefanv merged commit 90f0c3d into scikit-image:main Apr 28, 2025
24 checks passed
@lagru lagru deleted the use-num_workers branch April 28, 2025 16:31
matthew-brett added a commit to matthew-brett/scikit-image that referenced this pull request Jul 7, 2025
@matthew-brett
Merge remote-tracking branch 'origin/main' into refactor-fundamental
91f2233 
* origin/main:
  Deprecate estimate method in favor of class constructor (scikit-image#7771)
  Temporary fix for Visual Studio & Clang incompatibility in Windows image (scikit-image#7835)
  Address deprecations in Pillow 11.3 (scikit-image#7828)
  Remove unused & obsolete `legacy_datasets`, `legacy_registry` vars (scikit-image#7677)
  Draft migration guide for skimage2 (scikit-image#7785)
  Do not report failure in wheels sub-recipe (scikit-image#7806)
  Use consistent wording in property description. (scikit-image#7804)
  Add intensity_median to regionprops (scikit-image#7745)
  CI: Update pypa/gh-action-pypi-publish to v1.12.4 for attestations on PyPI (scikit-image#7793)
  Document output dtype for transform.resize. (scikit-image#7792)
  Use `cibuildwheel` to build WASM/Pyodide wheels for `scikit-image`, push nightlies to Anaconda.org (scikit-image#7440)
  DOC: Include missing gain parameter in adjust_gamma equation (scikit-image#7763)
  Temporarily pin to `pyodide-build==0.30.0`, and ensure that the correct xbuildenvs are used (scikit-image#7788)
  Deprecate old names & attributes in RegionProperties (scikit-image#7778)
  Pin JasonEtco/create-an-issue action to SHA for v2.9.2 (scikit-image#7787)
  Make doctest-plus work with spin (scikit-image#7786)
  Report failures on main via issue (scikit-image#7752)
  Use `workers` instead of alternate parameter names (scikit-image#7302)
  Fix f-string in otsu plot (scikit-image#7780)
  Further document use of regionprops function and fix terms. (scikit-image#7518)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stefanv stefanv stefanv approved these changes

@mkcor mkcor mkcor approved these changes

Assignees

No one assigned

Labels

🔽 Deprecation Involves deprecation ⬆️ Upstream Involves an upstream project 🥾 Path to skimage2 A step towards the new "API 2.0" 📜 type: API Involves API change(s)

Projects

None yet

Milestone

0.26

Development

Successfully merging this pull request may close these issues.

Terminology: n_jobs vs num_workers vs ncpu etc.

4 participants

@lagru @stefanv @jarrodmillman @mkcor