Comments (7)
I concur with @hmaarrfk.
In a vacuum option 1 seems most intuitive to me. However, it also seems like something that changes output for the same input. We have a policy not to do that. Even if an intermediate deprecation would warn most users, some might miss it and might have trouble reproducing their results down the line. As Mark hinted at, we plan to remove automatic scaling in most cases by default in "skimage2". I'll add this as a problem to address in API changes for skimage2.
In the meantime and in addition to documenting the behavior, we could raise a warning if the cval
is outside the value range of the rescaled image. If you would like to do a PR for this @idc9, feel welcome to request my review on it. :D
from scikit-image.
Thanks for the bug report. Indeed this is likely a confusing part of our API.
Perhaps an improvement to the documentation would suffice?
You can make a PR making modifications to
scikit-image/skimage/transform/_warps.py
Line 833 in 21485b1
And the added explination will appear in jupyter notebooks and our online documentation
from scikit-image.
Got it -- that all makes sense. I'll put in a PR when I get a chance to come back to this!
from scikit-image.
It might be worth changing the behavior of warp
instead of changing the documentation. The options I see are
- Change behavior so cval is automatically rescaled if the image is rescaled.
- Add an argument to give user the option to either automatically convert cval or to always preserve the input cval. Need to decide option would be the default behavior.
- Keep the current behavior where the cval is not changed even if the image values are. Change the documentation to make user aware of this issue.
Requiring the user to manually scale cval might not be a good idea. It may not occur to many people to do this. Also it's a little bit involved to get the scaling behavior correct. The solution I'm currently using is
cval_maybe_rescaled = img_as_float(np.array([cval]).astype(image.dtype)).item()
which is a little bit ugly/involved for the user to do on their own.
I would vote for option 1, but it might be worth getting other people's input on this.
Tradeoffs
Option 1
- pro: the user can safely enter the cval in the original scale and
warp
will handle possibly rescaling. - pro: no thought on the user's side, not option for user to do the wrong thing
- con: breaking change
Option 2
- con: introduces an additional argument
- pro: keep option for old vs. new behavior (not sure if this is usful?)
Option 2, default to automatic rescale
- pro: keeps pros of option 1
- con: breaking change
Option 2, default to current behavior with no rescale
- pro: no breaking change
- con: default option results in wrong/unintuitive behavior of cval
Option 3
- pro: no breaking change
- con: user has to manually scale the cval before passing into
warp
depending on what happens inside ofwarp
.
If we go with option 2/3 we could add a function to skimage def rescale_cval_to_float(cval, image)
that handles rescaling the cval.
from scikit-image.
I don't disagree with any of the core concepts you bring up, i'm just not sure a 1-2 year(+???) deprecation cycle is really what you want. The worst part for you, will likely be that you will have old code that requires an old version of scikit image (for whatever reason) that will just behave exactly in the way you do not expect. You'll end up writing your own compatibility shims to work with scikit-image 0.20.0 (1 year old) to today's version (0.22.0) through to the future versions where your proposals would end up being the default.
preserve_range
and units were a big challenge that we ultimately weren't able to resolve quickly enough (well 6 years now)
#3263
I'm trying to find where consensus along the core maintainers (myself included) has landed and the general idea is to set preserve_range=True
everywhere and make that the only option in skimage(next). Ultimately the problem is that with large datasets, that floating point conversion is just costly in terms of computation, and often times unexpected. Casting to float32 is might be costly, but the value of doing / 255
just to rescale again by some contrast adjustment code is often just silly.
This is one comment that hints at that, but we had a larger discussion somewhere
https://discuss.scientific-python.org/t/a-pragmatic-pathway-towards-skimage2/530/20
Option 1 and 2 are fine proposals, just given the bandwidth of the core maintainers, I don't think they will get implemented.
The line between bug and feature is quite thin in this particular example, unfortunately, documenting this quirk in the behavior might be the fastest and most effective approach for all parties.
from scikit-image.
Hello scikit-image core devs! There hasn't been any activity on this issue 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 issue fell through the cracks! Hopefully this ping will help bring some fresh attention to the issue. If you need help, you can always reach out on our forum
If you think that this issue is no longer relevant, you may close it, or we may do it at some point (either way, it will be done manually).
from scikit-image.
Not stale
from scikit-image.
Related Issues (20)
- Make sure `__array__(..., copy=True)` returns copies
- Setting up conda environment for development of skimage throws a package not found error HOT 5
- Backend QtAgg is interactive backend. Turning interactive mode on. HOT 4
- autosummary.import_cycle warning breaks doc build on Windows CI HOT 4
- Strange description in docstring
- `filters.threshold_triangle` fails for Dask 2024.8 with "unkown chunk size" HOT 2
- pytest-runner deprecated? HOT 1
- `test_ellipse_model_estimate_failers` fails on master for Mac M1 (arm64) HOT 7
- About the PyWavelets not installed error HOT 2
- Error in saving the resulting image of denoise wavelet HOT 2
- One of our doc dependencies brings in numpy < 2 HOT 3
- `io.show` only shows the last image on macOS (M2) HOT 2
- Module Not Found Error HOT 1
- Math display rendering is broken. HOT 1
- transform.resize does not necessarily return the same dtype as its input HOT 1
- Adding descriptor extractors given keypoints to `skimage.feature.SIFT` HOT 6
- Remove deprecated function usage from gallery HOT 2
- local_minima output wrong with numpy 2.0.2 HOT 3
- Measure region properties example has an error that misses hover over label for first mask HOT 2
- Polluted namespace by lazy loader
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from scikit-image.