Skip to content

Conversation

@Daraan
Copy link
Contributor

@Daraan Daraan commented Nov 29, 2025

Adds a generic TapIgnore - which is a Annotated[T, _TapIgnoreMarker] which ignores arguments.

As it is just a generic Annotated should work without issues for type-checkers.

Resolves: #75
Closes: #170, #92

Question:
For IDEs it would be better to use TypeAliasType for TapIgnore.tap itself avoids using typing_extensions but it is in the dependency chain from typing-inspect - so it would be usable. Not sure how your stance is to use it or not.

@codecov-commenter
Copy link

codecov-commenter commented Nov 29, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 93.93939% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.21%. Comparing base (2e18dc8) to head (dd5b883).

Files with missing lines Patch % Lines
src/tap/tap.py 90.00% 2 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #171      +/-   ##
==========================================
- Coverage   94.45%   94.21%   -0.24%     
==========================================
  Files           4        4              
  Lines         721      743      +22     
==========================================
+ Hits          681      700      +19     
- Misses         40       43       +3     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Daraan Daraan marked this pull request as ready for review November 29, 2025 14:16
@Daraan
Copy link
Contributor Author

Daraan commented Nov 29, 2025

Coverage skips two lines, they only way I see is to remove the double-check in _add_arguments - which is currently not necessary but I think they should be kept.

For the argument_buffer I am not sure If I can treat it the same.

@martinjm97 martinjm97 merged commit 0ced460 into swansonk14:main Dec 19, 2025
15 of 17 checks passed
@Daraan Daraan deleted the tap-ignore branch December 19, 2025 01:49
@martinjm97
Copy link
Collaborator

Hi @Daraan,

Thank you for this excellent PR! We have merged it. We did some additional testing d08ff5a and it seems that the current implementation does not support tapify at this point (existing behavior is that arguments labeled with TapIgnore seem to not be ignored). As a result, we raised the following issue #172.

Best,
JK

@Daraan Daraan restored the tap-ignore branch December 19, 2025 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

exclude variables annotated with ClassVar

3 participants