MAINT: remove vendored colorconv from skimage. #5180
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## main #5180 +/- ##
==========================================
+ Coverage 88.76% 88.94% +0.17%
==========================================
Files 579 571 -8
Lines 49113 48629 -484
==========================================
- Hits 43596 43253 -343
+ Misses 5517 5376 -141
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
andy-sweet
approved these changes
Oct 7, 2022
As it seem skimage is a dependency, I don't think we need to vendor this anymore. _if_ we vendor it, then we likely want to have a script that automatically check for updates – or make sure this file is not modified manually. Also if vendored, it is likely better placed in napari/_vendor, where everything is vendored.
|
Thanks. |
|
@Carreau Bisect pointed to this PR. |
|
Yeah, I sent the PR upstream that modified the warning stacklevel as the warning was silent. |
|
I see, so maybe we should open an issue here in napari to track down the source of the issue in labels and fix it. |
Yes, I think that would be a good thing, I will try to re-trigger it with main of skimage to actually find the correct place where incorrect data is passed in, and scikit-image/scikit-image#6613 will make it much easier. Unless you get to it first... |
|
So the warning in napari emanates from this line: I'm not familiar enough with lab and XYZ colorspace to understand why this is a problem, and what to do about it. |
2 tasks
jni
pushed a commit
that referenced
this pull request
Dec 14, 2022
…ates (#5386) Closes #5311 Our default Labels colormap is generated by sampling uniformly at random from the unit cube. These 3-tuples are intended to be coordinates in the CIELAB colorspace, but some of those coordinates may not be valid since that space is not linear. We then convert those coordinates to their corresponding RGB vectors. The scikit-image color conversion function handles this by projecting those invalid vectors, but it also issues a warning when doing so. This whole process approximates sampling uniformly at random over the CIELAB colorspace and the projection adds a little more error to the approximation, but overall it's good enough. Originally (in #175), we vendored code from scikit-image to handle color space conversion and removed this warning in our copy (see d1bb715). Later, we brought in scikit-image as a dependency. Then recently (in #5180) we removed the vendored code, which caused the warning to be issued (#5311). This PR specifically suppresses that warning. It also expands on the explanatory comment describing how the min and max values in the LAB and LUV spaces were precomputed.
melissawm
pushed a commit
to melissawm/napari
that referenced
this pull request
Jan 11, 2023
…ates (napari#5386) Closes napari#5311 Our default Labels colormap is generated by sampling uniformly at random from the unit cube. These 3-tuples are intended to be coordinates in the CIELAB colorspace, but some of those coordinates may not be valid since that space is not linear. We then convert those coordinates to their corresponding RGB vectors. The scikit-image color conversion function handles this by projecting those invalid vectors, but it also issues a warning when doing so. This whole process approximates sampling uniformly at random over the CIELAB colorspace and the projection adds a little more error to the approximation, but overall it's good enough. Originally (in napari#175), we vendored code from scikit-image to handle color space conversion and removed this warning in our copy (see d1bb715). Later, we brought in scikit-image as a dependency. Then recently (in napari#5180) we removed the vendored code, which caused the warning to be issued (napari#5311). This PR specifically suppresses that warning. It also expands on the explanatory comment describing how the min and max values in the LAB and LUV spaces were precomputed.
Czaki
added
maintenance
Czaki
pushed a commit
that referenced
this pull request
Jun 16, 2023
As it seem skimage is a dependency, I don't think we need to vendor this anymore. _if_ we vendor it, then we likely want to have a script that automatically check for updates – or make sure this file is not modified manually. Also if vendored, it is likely better placed in napari/_vendor, where everything is vendored.
Czaki
pushed a commit
that referenced
this pull request
Jun 17, 2023
…ates (#5386) Closes #5311 Our default Labels colormap is generated by sampling uniformly at random from the unit cube. These 3-tuples are intended to be coordinates in the CIELAB colorspace, but some of those coordinates may not be valid since that space is not linear. We then convert those coordinates to their corresponding RGB vectors. The scikit-image color conversion function handles this by projecting those invalid vectors, but it also issues a warning when doing so. This whole process approximates sampling uniformly at random over the CIELAB colorspace and the projection adds a little more error to the approximation, but overall it's good enough. Originally (in #175), we vendored code from scikit-image to handle color space conversion and removed this warning in our copy (see d1bb715). Later, we brought in scikit-image as a dependency. Then recently (in #5180) we removed the vendored code, which caused the warning to be issued (#5311). This PR specifically suppresses that warning. It also expands on the explanatory comment describing how the min and max values in the LAB and LUV spaces were precomputed.
Czaki
pushed a commit
that referenced
this pull request
Jun 17, 2023
As it seem skimage is a dependency, I don't think we need to vendor this anymore. _if_ we vendor it, then we likely want to have a script that automatically check for updates – or make sure this file is not modified manually. Also if vendored, it is likely better placed in napari/_vendor, where everything is vendored.
Czaki
pushed a commit
that referenced
this pull request
Jun 18, 2023
…ates (#5386) Closes #5311 Our default Labels colormap is generated by sampling uniformly at random from the unit cube. These 3-tuples are intended to be coordinates in the CIELAB colorspace, but some of those coordinates may not be valid since that space is not linear. We then convert those coordinates to their corresponding RGB vectors. The scikit-image color conversion function handles this by projecting those invalid vectors, but it also issues a warning when doing so. This whole process approximates sampling uniformly at random over the CIELAB colorspace and the projection adds a little more error to the approximation, but overall it's good enough. Originally (in #175), we vendored code from scikit-image to handle color space conversion and removed this warning in our copy (see d1bb715). Later, we brought in scikit-image as a dependency. Then recently (in #5180) we removed the vendored code, which caused the warning to be issued (#5311). This PR specifically suppresses that warning. It also expands on the explanatory comment describing how the min and max values in the LAB and LUV spaces were precomputed.
Czaki
pushed a commit
that referenced
this pull request
Jun 18, 2023
As it seem skimage is a dependency, I don't think we need to vendor this anymore. _if_ we vendor it, then we likely want to have a script that automatically check for updates – or make sure this file is not modified manually. Also if vendored, it is likely better placed in napari/_vendor, where everything is vendored.
Czaki
pushed a commit
that referenced
this pull request
Jun 19, 2023
…ates (#5386) Closes #5311 Our default Labels colormap is generated by sampling uniformly at random from the unit cube. These 3-tuples are intended to be coordinates in the CIELAB colorspace, but some of those coordinates may not be valid since that space is not linear. We then convert those coordinates to their corresponding RGB vectors. The scikit-image color conversion function handles this by projecting those invalid vectors, but it also issues a warning when doing so. This whole process approximates sampling uniformly at random over the CIELAB colorspace and the projection adds a little more error to the approximation, but overall it's good enough. Originally (in #175), we vendored code from scikit-image to handle color space conversion and removed this warning in our copy (see d1bb715). Later, we brought in scikit-image as a dependency. Then recently (in #5180) we removed the vendored code, which caused the warning to be issued (#5311). This PR specifically suppresses that warning. It also expands on the explanatory comment describing how the min and max values in the LAB and LUV spaces were precomputed.
Czaki
pushed a commit
that referenced
this pull request
Jun 19, 2023
As it seem skimage is a dependency, I don't think we need to vendor this anymore. _if_ we vendor it, then we likely want to have a script that automatically check for updates – or make sure this file is not modified manually. Also if vendored, it is likely better placed in napari/_vendor, where everything is vendored.
Czaki
pushed a commit
that referenced
this pull request
Jun 21, 2023
…ates (#5386) Closes #5311 Our default Labels colormap is generated by sampling uniformly at random from the unit cube. These 3-tuples are intended to be coordinates in the CIELAB colorspace, but some of those coordinates may not be valid since that space is not linear. We then convert those coordinates to their corresponding RGB vectors. The scikit-image color conversion function handles this by projecting those invalid vectors, but it also issues a warning when doing so. This whole process approximates sampling uniformly at random over the CIELAB colorspace and the projection adds a little more error to the approximation, but overall it's good enough. Originally (in #175), we vendored code from scikit-image to handle color space conversion and removed this warning in our copy (see d1bb715). Later, we brought in scikit-image as a dependency. Then recently (in #5180) we removed the vendored code, which caused the warning to be issued (#5311). This PR specifically suppresses that warning. It also expands on the explanatory comment describing how the min and max values in the LAB and LUV spaces were precomputed.
Czaki
pushed a commit
that referenced
this pull request
Jun 21, 2023
As it seem skimage is a dependency, I don't think we need to vendor this anymore. _if_ we vendor it, then we likely want to have a script that automatically check for updates – or make sure this file is not modified manually. Also if vendored, it is likely better placed in napari/_vendor, where everything is vendored.
Czaki
pushed a commit
that referenced
this pull request
Jun 21, 2023
…ates (#5386) Closes #5311 Our default Labels colormap is generated by sampling uniformly at random from the unit cube. These 3-tuples are intended to be coordinates in the CIELAB colorspace, but some of those coordinates may not be valid since that space is not linear. We then convert those coordinates to their corresponding RGB vectors. The scikit-image color conversion function handles this by projecting those invalid vectors, but it also issues a warning when doing so. This whole process approximates sampling uniformly at random over the CIELAB colorspace and the projection adds a little more error to the approximation, but overall it's good enough. Originally (in #175), we vendored code from scikit-image to handle color space conversion and removed this warning in our copy (see d1bb715). Later, we brought in scikit-image as a dependency. Then recently (in #5180) we removed the vendored code, which caused the warning to be issued (#5311). This PR specifically suppresses that warning. It also expands on the explanatory comment describing how the min and max values in the LAB and LUV spaces were precomputed.
Czaki
pushed a commit
that referenced
this pull request
Jun 21, 2023
As it seem skimage is a dependency, I don't think we need to vendor this anymore. _if_ we vendor it, then we likely want to have a script that automatically check for updates – or make sure this file is not modified manually. Also if vendored, it is likely better placed in napari/_vendor, where everything is vendored.
Czaki
pushed a commit
that referenced
this pull request
Jun 21, 2023
…ates (#5386) Closes #5311 Our default Labels colormap is generated by sampling uniformly at random from the unit cube. These 3-tuples are intended to be coordinates in the CIELAB colorspace, but some of those coordinates may not be valid since that space is not linear. We then convert those coordinates to their corresponding RGB vectors. The scikit-image color conversion function handles this by projecting those invalid vectors, but it also issues a warning when doing so. This whole process approximates sampling uniformly at random over the CIELAB colorspace and the projection adds a little more error to the approximation, but overall it's good enough. Originally (in #175), we vendored code from scikit-image to handle color space conversion and removed this warning in our copy (see d1bb715). Later, we brought in scikit-image as a dependency. Then recently (in #5180) we removed the vendored code, which caused the warning to be issued (#5311). This PR specifically suppresses that warning. It also expands on the explanatory comment describing how the min and max values in the LAB and LUV spaces were precomputed.
Czaki
pushed a commit
that referenced
this pull request
Jun 21, 2023
As it seem skimage is a dependency, I don't think we need to vendor this anymore. _if_ we vendor it, then we likely want to have a script that automatically check for updates – or make sure this file is not modified manually. Also if vendored, it is likely better placed in napari/_vendor, where everything is vendored.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As it seem skimage is a dependency, I don't think we need to vendor this anymore.
if we vendor it, then we likely want to have a script that automatically check for updates – or make sure this file is not modified manually.
Also if vendored, it is likely better placed in napari/_vendor, where everything is vendored.
Description
Type of change
References
How has this been tested?
as there are small differences between the two Qt bindings.
Final checklist:
trans.to make them localizable.For more information see our translations guide.