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

Handle None types #742

Merged
merged 9 commits into from
Apr 26, 2024
Merged

Handle None types #742

merged 9 commits into from
Apr 26, 2024

Conversation

tclose
Copy link
Contributor

@tclose tclose commented Mar 25, 2024

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Summary

  • Handles None in field types, e.g. int | None instead of typing.Optional[int]

Checklist

  • I have added tests to cover my changes (if necessary)
  • I have updated documentation (if necessary)

added by @ghisvail
Closes #744

@tclose tclose marked this pull request as draft March 25, 2024 02:27
@tclose tclose changed the title adds bytes repr implementation for mock fileset types to get pydra task doctests to work Handle None types Mar 25, 2024
Copy link

codecov bot commented Mar 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.71%. Comparing base (e52e32b) to head (a7ac191).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #742      +/-   ##
==========================================
+ Coverage   84.27%   91.71%   +7.43%     
==========================================
  Files          26       55      +29     
  Lines        5127    15681   +10554     
  Branches     1447     2792    +1345     
==========================================
+ Hits         4321    14382   +10061     
- Misses        800     1291     +491     
- Partials        6        8       +2     
Flag Coverage Δ
unittests 91.71% <100.00%> (+7.43%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

pydra/utils/typing.py Outdated Show resolved Hide resolved
@ghisvail
Copy link
Collaborator

ghisvail commented Apr 3, 2024

I did a superficial read of the diff and what I checked so far looked sensible 👍

Comment on lines +210 to +213
@pytest.mark.skipif(sys.version_info < (3, 10), reason="No UnionType < Py3.10")
def test_type_check_fail2a():
with pytest.raises(TypeError, match="to any of the union types"):
TypeParser(Path | File)(lz(int))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it possible to run this test with a from __future__ import annotations for Python < 3.10?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from __future__ import annotations just leaves all annotations as strings I believe. Probably need to have a separate test module to handle that case, but since it doesn't look like that will be the way that types are handled going forward I have tended to avoid it

@ghisvail ghisvail merged commit 0af07fb into master Apr 26, 2024
43 checks passed
@ghisvail ghisvail deleted the none-type-typing branch April 26, 2024 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants