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

"format" supports all types by default #102

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

handrews
Copy link
Contributor

I attempted to implement numeric formats for oascomply, and it took quite a lot of effort to figure out why they just wouldn't work.

Previously, formats could set applicable types, but in the decorator the types were defaulted to ("string",), contrary to the behavior of keywords in general and the "format" keyword class in particular.

Since the tutorial example, unit tests, and the one format validator all check the instance type before proceeding, it looked very much like format validator functions were expected to do their own type checking.

This change aligns the default type applicability of formats with that of keywords, and updates all implementations, tests, and examples to rely on the decorator parameter to handle the type check.

I wrote the CHANGELOG as an 0.10.4 (thanks so much for the quick 0.10.3!), but I can see the argument that this is a public interface change.

@codecov
Copy link

codecov bot commented May 22, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.04 🎉

Comparison is base (3ff123b) 92.62% compared to head (e7a7279) 92.66%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #102      +/-   ##
==========================================
+ Coverage   92.62%   92.66%   +0.04%     
==========================================
  Files          23       23              
  Lines        2033     2032       -1     
  Branches      428      427       -1     
==========================================
  Hits         1883     1883              
  Misses         97       97              
+ Partials       53       52       -1     
Impacted Files Coverage Δ
jschon/formats.py 100.00% <100.00%> (+14.28%) ⬆️
jschon/vocabulary/format.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Previously, formats could set applicable types, but in the decorator
the types were defaulted to ("string",), contrary to the behavior of
keywords in general and the "format" keyword class in particular.

Since the tutorial example, unit tests, and the one format
validator all check the instance type before proceeding, it looked
very much like format validator functions were expected to do
their own type checking.

This change aligns the default type applicability of formats with
that of keywords, and updates all implementations, tests, and
examples to rely on the decorator parameter to handle the type check.
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.

1 participant