Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix safe_issubclass for NewType #678

Merged
merged 6 commits into from
Nov 23, 2024
Merged

Conversation

Czaki
Copy link
Contributor

@Czaki Czaki commented Nov 22, 2024

Closes napari/napari#7129

Today I learn that such code:

new_int = NewType("new_int", int)

issubclass(new_int, new_int)

Raises exception instead of returning True.

So this PR is a followup to #665 to fully resolve the upstream napari bug.

@Czaki Czaki requested a review from tlambert03 November 22, 2024 21:22
Copy link

codecov bot commented Nov 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.85%. Comparing base (021654f) to head (fbe381b).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #678      +/-   ##
==========================================
- Coverage   88.87%   88.85%   -0.02%     
==========================================
  Files          39       39              
  Lines        4754     4756       +2     
==========================================
+ Hits         4225     4226       +1     
- Misses        529      530       +1     

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

@Czaki
Copy link
Contributor Author

Czaki commented Nov 22, 2024

@tlambert03 The #671 changed type annotation used in tests/test_util.py making many test repetitive.
Should we restore these changes or simplify tests?

src/magicgui/_util.py Outdated Show resolved Hide resolved
@tlambert03
Copy link
Member

Should we restore these changes or simplify tests?

ah, i think these should probably be restored. since typing.List is still not an alias for list (in fact they don't even equal each other), it probably doesn't hurt to just keep checking that the typing objects work explicitly

@tlambert03 tlambert03 merged commit 25a2a73 into main Nov 23, 2024
36 checks passed
@tlambert03 tlambert03 deleted the fix_list_layerdata_tuple branch November 23, 2024 21:38
@tlambert03
Copy link
Member

Thanks!

@tlambert03
Copy link
Member

Do you need this released asap? Can it wait a week or so to give time for testing the current pre-release? Or would you prefer a cherry pick back-release

@Czaki
Copy link
Contributor Author

Czaki commented Nov 23, 2024

This is a question to @psobolewskiPhD

@psobolewskiPhD
Copy link
Contributor

I mean it's been around and it only affects things that didn't update to drop older python, so I don't think it's critical?
I think being able to update the image.sc thread that it's been fixed and say look for a new magicgui release in a week or 2 should be fine?

@tlambert03
Copy link
Member

Sounds good thanks

@imagesc-bot
Copy link

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/help-with-napari-unicell-plugin/105331/15

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Widgets with return type annotation List[LayerDataTuple] don't get their layers added to the Viewer
5 participants