search

napari / napari

napari: a fast, interactive, multi-dimensional image viewer for python
https://napari.org
BSD 3-Clause "New" or "Revised" License
2.1k stars 410 forks source link

DeprecationWarning in ensure_iterable and is_iterable, Closes #6256 #6836

Closed palec87 closed 2 weeks ago

palec87 commented 2 months ago

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.

Carreau commented 1 month ago

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 commented 1 month ago

@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 commented 1 month ago

@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 commented 1 month ago

Great! What timezone are you in?

palec87 commented 1 month ago

Great! What timezone are you in?

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

melonora commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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

melonora commented 1 month ago

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

palec87 commented 1 month ago

Sure, please push the changes.

melonora commented 1 month ago

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 commented 1 month ago

if you want we can have a look together

palec87 commented 1 month ago

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 commented 1 month ago

@palec87 Would you ahve time Friday?

palec87 commented 1 month ago

@palec87 Would you ahve time Friday?

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

palec87 commented 3 weeks ago

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[bot] commented 3 weeks ago

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 :warning:
Additional details and impacted files ```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.