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

Implement Predicate decorator #146

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

Conversation

doloopwhile
Copy link

Hello. I added Predicate decorator.

Currently, To define a functional schema, We have to write a def statement and an assignment statement.

def some_condition(x):
   ...
   ...
   (Some validation logic longer than 10 lines)
   ...
   ...

some_condition.__name__ = "Error message" # When define it as naked function
some_condition = Schema(some_condition, error="Error message") # When define it as object

Predicate make it possible to define a functional schema as one def statement.

@Predicate(error="Error message")
def some_condition(x):
   ...
   ...

I feel later style is more readable (in the same reason of @getter defining properties).

@skorokithakis
Copy link
Collaborator

Thank you for your contribution! Hmm, this looks useful, but I don't know if the verbose way is currently so big a pain to justify including this predicate in the library. On the other hand, it can't hurt, but it does make the library a bit more complicated. You're literally replacing this:

some_condition = Schema(some_condition, error="Error message")

with this:

@Predicate(error="Error message")

correct? The latter is more readable, true, but I'm not sure that's not enough justification to include this. Does anyone else want to chime in?

@codecov-io
Copy link

Codecov Report

Merging #146 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #146   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      1           
  Lines         208    212    +4     
=====================================
+ Hits          208    212    +4
Impacted Files Coverage Δ
schema.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2eeccbe...730d978. Read the comment docs.

Copy link

@gst gst left a comment

Choose a reason for hiding this comment

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

the fact is that there are 2 different things, the schema (or condition validator) and the function condition itself.. one might reuse the condition in other places and thus the decorator is ineffective given the behavior is changed ; the resulting schema can't be called as if it was the condition..
but the idea is still interesting I think.

@Predicate("ERROR2", ignore_extra_keys=True)
def p2(x):
return x == 2
assert p2._error is "ERROR2" and p2._ignore_extra_keys
Copy link

Choose a reason for hiding this comment

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

you should use == to compare strings

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.

4 participants