Codecov Report

Merging #45 (0e08eca) into main (0ad1bd8) will increase coverage by 0.24%.
The diff coverage is 96.15%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main      #45      +/-   ##
==========================================
+ Coverage   94.25%   94.50%   +0.24%     
==========================================
  Files           2        2              
  Lines         174      200      +26     
  Branches       27       29       +2     
==========================================
+ Hits          164      189      +25     
- Misses          8        9       +1     
  Partials        2        2              

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

I'd favor runtime checks (if debug:) that (a) whatever Not[...] wraps is a callable and not an instance of BaseMetadata, and (b) that whatever non-Predicate BaseMetadata classes wrap is not a Not. Seems kinda silly, but it's the kind of thing that can avoid a surprising amount of pain later. Perf impact seems small, but maybe we really care about it anyway 🤔

Do you mean that we check to prohibit Not[Predicate[...]] and only allow Predicate[Not[...]]?

Either of those seem fine, but I'd ban eg Not[Ge(5)] and MinLen([Not[3]).

MinLen([Not[3])

Won't this already be rejected by type checkers? Not requires a callable and 3 is not a callable. Similarly MinLen requires an int as the first argument and Not is not anint.

Yeah, seems fine to rely on static type checking if we expect users to have that.

I think that’s a fair bet and I’d rather rely on that than adding more runtime type checking here.