DeprecationWarning in ensure_iterable and is_iterable, Closes #6256 #6836

palec87 · 2024-04-13T16:40:10Z

References and relevant issues

Supposed to Close #6256. I am not sure about the stacklevel arg of the warning.

Description

Added DeprecationWarning if arguments passed and removed the functionality related to them within the functions.

Regarding checklist

I am not sure about the docs repo.

…terable

for more information, see https://pre-commit.ci

Carreau · 2024-04-15T08:10:36Z

napari/utils/misc.py

-    can either be a single value or a list. If a color is passed then it
-    will be treated specially to determine if it is iterable.
+    can either be a single value or a list.
+    Argument color is deprecated.
    """
-    if is_iterable(arg, color=color):
+    # deprecate color
+    if color:
+        warnings.warn(
+            trans._(
+                'Argument color is deprecated.',
+            ),
+            category=DeprecationWarning,
+            stacklevel=2,  # not sure what level to use here
+        )
+    if is_iterable(arg):


You likely want to do something like:

sentinel = object() def ensure_iterabel(...., color = sentinel): if color is not sentinel: warning.warn...

This way the warning is triggered even if color=False is passed.

another way is to do

def ensure_iterable(..., *args, **kwargs):

and then check the content of args and kwags, but it's a bit more verbose.

You likely also want to say either since when it's deprecated, and/or when it will be removed I think.

I'm not that involved in the project anymore but other dev will likely pitch in. Check with them.

Thanks for the draft PR @palec87

Agree, though I think we would need to make the decision on when it would be deprecated. Given that it is not used this could perhaps be sooner rather than later (0.5.0). What do you think @Czaki?

Deprecation warnings have to be released at least once so people can see them, so we can't deprecate in 0.5.0!

Deprecation warnings have to be released at least once so people can see them, so we can't deprecate in 0.5.0!

Are yon confusing Deprecation and Removal ?

I mean most of the "Deprecated in X" are emitted for the first when doing release X...

Targeted for deprecation from 0.5.0, but then deprecated before 0.6.0 given that these are not used.

Sorry, indeed getting confused, ignore my brainfart :P Yes, deprecate in 0.5.0 is good!

Thank you @Carreau, I have already learned something new. @melonora, with my new commits, if I understand the deprecation logic, I returned the original color/allow_none argument functionality and only marked it for removal in 0.6.0. It does not change the runtime, because it is not used, however it should be removed only after 0.5.0 release, right?

Thanks @palec87, it seems good to me. The stacklevel determines how many calls to unwind so it is good not to get this number too high.

…ocstrings with deprecation in 0.5.0 and removel in 0.6.0. Put back the color functionality

Carreau · 2024-04-22T08:54:18Z

Thanks, the new logic looks good to me – but I not a core dev so it's not my role to decide if this is ok.

I would mark this as ready for review. This seem to have picked up conflicts with the main branch.

@palec87 are you familiar with merging with/rebasing on main and fixing merge conflicts ?

melonora · 2024-05-01T06:49:40Z

@palec87 As @Carreau pointed out there are some conflicts. If you would like you can resolve them and get the PR out of draft mode. If you are not familiar with resolving conflicts I would also be happy to have a small call with you to show you.

palec87 · 2024-05-02T07:57:48Z

@palec87 As @Carreau pointed out there are some conflicts. If you would like you can resolve them and get the PR out of draft mode. If you are not familiar with resolving conflicts I would also be happy to have a small call with you to show you.

Hi @melonora, I would like to go with the call and not messing up. Perhaps tomorrow 3/5? Thanks for help @Carreau too, very appreciated.

melonora · 2024-05-02T09:11:11Z

Great! What timezone are you in?

palec87 · 2024-05-02T11:15:59Z

Great! What timezone are you in?

I am UTC+1, free untill lunch for sure.

for more information, see https://pre-commit.ci

melonora · 2024-05-08T10:04:35Z

Hi @palec87, I see that some tests are failing due to the deprecation warning. I am implementing some minor fixes for those. These are not related to the merge.

melonora · 2024-05-08T11:10:59Z

however, they are related to the changes. Although at a first glance it seems that allow_none and color have no effect it has some downstream effects on the shapes layer that are not yet all too clear to me.

I tested this after fixing the deprecation warning errors.

palec87 · 2024-05-08T12:00:47Z

however, they are related to the changes. Although at a first glance it seems that allow_none and color have no effect it has some downstream effects on the shapes layer that are not yet all too clear to me.

I tested this after fixing the deprecation warning errors.

Hi @melonora, I see, do you think it is because of the Deprecation warning? Your testing means running the pytest locally, or something else? To me it looks like test_ problem, not shapes layer probles. However, I did not see the -W option for pytest which would result in such errors.

melonora · 2024-05-08T12:02:14Z

I can push fixes for the deprecation warning and then the actual errors will show. Is it ok if I do so?

melonora · 2024-05-08T12:02:41Z

Otherwise I can schedule a short call again and we can go through it together.

palec87 · 2024-05-08T12:05:04Z

Sure, please push the changes.

melonora · 2024-05-08T12:37:38Z

Ok now you see the actual problem. Particularly the problem occurs here:
https://github.com/palec87/napari/blob/7e558aaec49d5a112e477f51218a512b42d31af1/napari/layers/shapes/shapes.py#L2276-L2312

melonora · 2024-05-08T12:38:00Z

if you want we can have a look together

palec87 · 2024-05-08T12:47:54Z

if you want we can have a look together

I see know, pretty deep for me, but I have 30 mins, if you want to show me.

melonora · 2024-05-15T09:32:04Z

@palec87 Would you ahve time Friday?

palec87 · 2024-05-15T09:47:01Z

@palec87 Would you ahve time Friday?

Hi @melonora , yes I think so. Ping me on LinkIn?

…y as True. Fixed conditional accounting for color being an object() posssibly, not only bool

palec87 · 2024-05-20T16:05:01Z

Hello @melonora, here is my fix. I removed deprecation for allow_none, because it is being set True in stack_utils.py on line 121. Shapes tests were failing because of conditionals when color has been set as object(). Now tests are passing locally.

codecov · 2024-05-20T18:32:16Z

Codecov Report

Attention: Patch coverage is 80.00000% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 92.43%. Comparing base (2102518) to head (96967a9).

Files	Patch %	Lines
napari/utils/misc.py	80.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6836      +/-   ##
==========================================
+ Coverage   92.31%   92.43%   +0.11%     
==========================================
  Files         613      613              
  Lines       55176    55180       +4     
==========================================
+ Hits        50936    51003      +67     
+ Misses       4240     4177      -63

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

melonora

LGTM now!

remove comment

…6256 (napari#6836) # References and relevant issues Supposed to Close napari#6256. I am not sure about the stacklevel arg of the warning. # Description Added DeprecationWarning if arguments passed and removed the functionality related to them within the functions. # Regarding checklist I am not sure about the docs repo. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Wouter-Michiel Vierdag <michiel.vierdag@embl.de> Co-authored-by: Wouter-Michiel Vierdag <w-mv@hotmail.com>

palec87 and others added 3 commits April 13, 2024 17:27

deprecated color and allow_none arguments in ensure_iterable and is_i…

f908786

…terable

fix docstring

0de7398

[pre-commit.ci] auto fixes from pre-commit.com hooks

679920c

for more information, see https://pre-commit.ci

Carreau reviewed Apr 15, 2024

View reviewed changes

palec87 added 3 commits April 19, 2024 10:16

introduce _sentinel to check passing argument, updated warnings and d…

4c20372

…ocstrings with deprecation in 0.5.0 and removel in 0.6.0. Put back the color functionality

Merge branch 'main' of https://github.com/palec87/napari

0fb2c97

returning allow_none functionality.

0234627

Carreau approved these changes Apr 22, 2024

View reviewed changes

palec87 marked this pull request as ready for review May 3, 2024 08:11

palec87 and others added 2 commits May 3, 2024 09:14

Merge branch 'main' into main

5151659

[pre-commit.ci] auto fixes from pre-commit.com hooks

478dd14

for more information, see https://pre-commit.ci

fix most tests failing with deprecationwarning

7e558aa

github-actions bot added the tests Something related to our tests label May 8, 2024

DragaDoncila mentioned this pull request May 15, 2024

Add deprecation message to CallDefault function partly resolves issue#6257 #6901

Merged

5 tasks

remove deprecation of allow_none, because it is used in stack_utils.p…

0465008

…y as True. Fixed conditional accounting for color being an object() posssibly, not only bool

melonora and others added 2 commits May 20, 2024 22:11

Merge branch 'main' into main

dd3095a

remove = file

9a9f263

melonora approved these changes May 20, 2024

View reviewed changes

melonora added the ready to merge Last chance for comments! Will be merged in ~24h label May 20, 2024

melonora added this to the 0.5.0 milestone May 20, 2024

melonora added the maintenance PR with maintance changes, label May 20, 2024

Update test_misc.py

96967a9

remove comment

melonora merged commit 220d3e1 into napari:main May 26, 2024
34 checks passed

github-actions bot removed the ready to merge Last chance for comments! Will be merged in ~24h label May 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DeprecationWarning in ensure_iterable and is_iterable, Closes #6256 #6836

DeprecationWarning in ensure_iterable and is_iterable, Closes #6256 #6836

palec87 commented Apr 13, 2024

Carreau Apr 15, 2024

melonora Apr 18, 2024

brisvag Apr 18, 2024

Carreau Apr 18, 2024

melonora Apr 18, 2024

brisvag Apr 18, 2024

palec87 Apr 19, 2024 •

edited

melonora May 1, 2024

Carreau commented Apr 22, 2024

melonora commented May 1, 2024

palec87 commented May 2, 2024

melonora commented May 2, 2024

palec87 commented May 2, 2024

melonora commented May 8, 2024

melonora commented May 8, 2024

palec87 commented May 8, 2024

melonora commented May 8, 2024 •

edited

melonora commented May 8, 2024

palec87 commented May 8, 2024

melonora commented May 8, 2024

melonora commented May 8, 2024

palec87 commented May 8, 2024

melonora commented May 15, 2024

palec87 commented May 15, 2024

palec87 commented May 20, 2024

codecov bot commented May 20, 2024 •

edited

melonora left a comment

DeprecationWarning in ensure_iterable and is_iterable, Closes #6256 #6836

DeprecationWarning in ensure_iterable and is_iterable, Closes #6256 #6836

Conversation

palec87 commented Apr 13, 2024

References and relevant issues

Description

Regarding checklist

Carreau Apr 15, 2024

Choose a reason for hiding this comment

melonora Apr 18, 2024

Choose a reason for hiding this comment

brisvag Apr 18, 2024

Choose a reason for hiding this comment

Carreau Apr 18, 2024

Choose a reason for hiding this comment

melonora Apr 18, 2024

Choose a reason for hiding this comment

brisvag Apr 18, 2024

Choose a reason for hiding this comment

palec87 Apr 19, 2024 • edited

Choose a reason for hiding this comment

melonora May 1, 2024

Choose a reason for hiding this comment

Carreau commented Apr 22, 2024

melonora commented May 1, 2024

palec87 commented May 2, 2024

melonora commented May 2, 2024

palec87 commented May 2, 2024

melonora commented May 8, 2024

melonora commented May 8, 2024

palec87 commented May 8, 2024

melonora commented May 8, 2024 • edited

melonora commented May 8, 2024

palec87 commented May 8, 2024

melonora commented May 8, 2024

melonora commented May 8, 2024

palec87 commented May 8, 2024

melonora commented May 15, 2024

palec87 commented May 15, 2024

palec87 commented May 20, 2024

codecov bot commented May 20, 2024 • edited

Codecov Report

melonora left a comment

Choose a reason for hiding this comment

palec87 Apr 19, 2024 •

edited

melonora commented May 8, 2024 •

edited

codecov bot commented May 20, 2024 •

edited