Add a Maybe validator #232

AndreaCrotti · 2016-10-10T09:23:55Z

Simple Maybe validator, which encapsulates the fact that something can be None or of the given type

AndreaCrotti · 2016-10-10T09:25:23Z

It's quite a standard constructor in functional languages, I think it might be useful here as well since it's a common case for functions to return None in some conditions..

coveralls · 2016-10-10T09:26:53Z

Coverage increased (+0.03%) to 94.954% when pulling ca06496 on AndreaCrotti:add_maybe_constructor into 0c5a292 on alecthomas:master.

coveralls · 2016-10-10T09:26:53Z

Coverage increased (+0.03%) to 94.954% when pulling ca06496 on AndreaCrotti:add_maybe_constructor into 0c5a292 on alecthomas:master.

tusharmakkar08

Please resolve the comments.

tusharmakkar08 · 2016-10-10T09:28:55Z

voluptuous/tests/tests.py

@@ -544,6 +544,19 @@ def fn(arg):
    fn(1)


+def test_schema_decorator_no_args():
+    @validate(int, __return__=Maybe(int))


Can you add test case for Invalid input as well?

tusharmakkar08 · 2016-10-10T09:37:29Z

voluptuous/validators.py

@@ -459,6 +459,14 @@ def PathExists(v):
        raise PathInvalid("Not a Path")


+class Maybe(object):


Add __repr__ and docstring.

AndreaCrotti · 2016-10-10T09:39:15Z

Yes sure @tusharmakkar08 I will solve the issues, just wanted to know if you were happy to add this at all before I made the PR fully finished...
Is that something you would be happy to add then?

tusharmakkar08 · 2016-10-10T09:44:33Z

Yeah, Maybe sounds like a decent validator which incorporates None or the type which is specified in the schema. Although it can be done using Any but I believe Maybe addition would make it more verbose and user-friendly.

alecthomas · 2016-10-10T09:48:57Z

Is this any different from Any(T, None)?

If not, I'm -1 on this as it adds only very minor convenience.

tusharmakkar08 · 2016-10-10T09:51:37Z

AFAIK it's same as Any(T, None) just more verbose. @AndreaCrotti : Please confirm this.

AndreaCrotti · 2016-10-10T12:02:23Z

Well in terms of the result I guess it's the same thing, but:

Maybe is a constructor that everyone coming from functional languages will understand and be used to.
Maybe is in theory much more restrictive, and in because of this more clear what it actually does.
Since in theory if you pass a type it just means that the value can be of the given type or None, nothing else, while with Any you can pass any callable.

We can write Maybe using Any if you prefer, but I generally think it would be a nice thing to add..

AndreaCrotti · 2016-10-12T22:05:25Z

Any news about this @alecthomas ?

I'm happy to implement it using Any internally if you prefer and add the missing tests/documentation..

alecthomas · 2016-10-17T13:52:48Z

Sure, why not. Pretty minor change but if it helps with clarity it is probably a good idea.

coveralls · 2016-10-17T15:58:52Z

Coverage increased (+0.03%) to 94.954% when pulling 0f84edd on AndreaCrotti:add_maybe_constructor into 0c5a292 on alecthomas:master.

coveralls · 2016-10-17T16:11:39Z

Coverage increased (+0.04%) to 94.964% when pulling 77086ae on AndreaCrotti:add_maybe_constructor into 0c5a292 on alecthomas:master.

coveralls · 2016-10-17T16:25:35Z

Coverage increased (+0.04%) to 94.964% when pulling c2e01a1 on AndreaCrotti:add_maybe_constructor into 0c5a292 on alecthomas:master.

coveralls · 2016-10-17T16:25:35Z

Coverage increased (+0.04%) to 94.964% when pulling c2e01a1 on AndreaCrotti:add_maybe_constructor into 0c5a292 on alecthomas:master.

AndreaCrotti · 2016-10-18T20:26:25Z

@tusharmakkar08 @alecthomas I've done a few changes, can you let me know what you think now?

strangely I wanted to add a test like this:

    assert_equal(repr(s), "Maybe(<class 'int'>)")

but for some reason it doesn't work, even if I get exactly that trying from ipython..

coveralls · 2016-10-18T20:27:12Z

Coverage decreased (-0.05%) to 94.882% when pulling 2fa388a on AndreaCrotti:add_maybe_constructor into 0c5a292 on alecthomas:master.

AndreaCrotti · 2016-10-18T20:27:47Z

Also I've added a check to make sure that what you can only pass an actual type.

I could actually use the @Validate decorator if you are happy about that.

tusharmakkar08 · 2016-10-19T07:10:54Z

It's surprising that __repr__ test isn't working. I will try to look into it maybe later today. Feel free to use @validate decorator there.

AndreaCrotti · 2016-10-19T20:22:07Z

Very strangely if I try to do this

class Maybe(object):
    @validate(arg=type)
    def __init__(self, kind, msg=None):
        self.kind = kind

I get a very strange error

----------------------------------------------------------------------
File "/home/andrea/projects/oss_projects/python/voluptuous/README.md", line 568, in README.md
Failed example:
    str(exc) == "not a valid value @ data[0][0]"
Expected:
    True
Got:
    False
----------------------------------------------------------------------
File "/home/andrea/projects/oss_projects/python/voluptuous/README.md", line 578, in README.md
Failed example:
    schema([6])
Exception raised:
    Traceback (most recent call last):
      File "/usr/lib/python3.5/doctest.py", line 1321, in __run
        compileflags, 1), test.globs)
      File "<doctest README.md[106]>", line 1, in <module>
        schema([6])
    NameError: name 'schema' is not defined


----------------------------------------------------------------------

AndreaCrotti · 2016-10-19T20:23:04Z

I can also leave it like this otherwise if you are happy with it, and add a bit of documentation in case it's all good to go

tusharmakkar08 · 2016-10-21T08:59:20Z

Hey @AndreaCrotti

I would ideally like to have a test case with the __repr__ test. Once you get that working, please squash your commits and we would be good to go.

Thanks.

coveralls · 2016-10-21T10:05:07Z

Coverage decreased (-3.3%) to 91.616% when pulling fea2d4d on AndreaCrotti:add_maybe_constructor into 0c5a292 on alecthomas:master.

coveralls · 2016-10-21T10:05:07Z

Coverage decreased (-3.3%) to 91.616% when pulling fea2d4d on AndreaCrotti:add_maybe_constructor into 0c5a292 on alecthomas:master.

AndreaCrotti · 2016-10-21T10:06:57Z

I've done the repr test but it's a bit trivial now, because to support both py2 and py3 and fix this error

FAIL: Verify that __repr__ returns valid Python expressions
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/virtualenv/python3.2.6/lib/python3.2/site-packages/nose/case.py", line 198, in runTest
    self.test(*self.arg)
  File "/home/travis/build/alecthomas/voluptuous/voluptuous/tests/tests.py", line 384, in test_repr
    assert_equal(repr(maybe_int), "Maybe(<type 'int'>)")
AssertionError: "Maybe(<class 'int'>)" != "Maybe(<type 'int'>)"
- Maybe(<class 'int'>)
?        ^^^^^
+ Maybe(<type 'int'>)
?        ^^^^

I had to just repr(int) inside the test itself

coveralls · 2016-10-21T10:08:49Z

Coverage increased (+0.06%) to 94.99% when pulling 5dfa2b2 on AndreaCrotti:add_maybe_constructor into 0c5a292 on alecthomas:master.

AndreaCrotti · 2016-10-21T10:09:11Z

@tusharmakkar08 @alecthomas what do you think better now?

AndreaCrotti · 2016-10-24T13:16:08Z

@tusharmakkar08 that's not related at all I believe..
I'm not changing behaviour depending on the python version, just the actual output from the test changes, so that doesn't really apply..

divanovGH · 2016-10-25T16:23:55Z

voluptuous/tests/tests.py

+
+    s = Schema(Maybe(int))
+    assert s(1) == 1
+


Where is your test for: assert s(None) == None ?

divanovGH · 2016-10-25T16:26:20Z

voluptuous/validators.py

+class Maybe(object):
+    """Validate that the object is of a given type or is None.
+
+    :raises Invalid: if the value is not of the type declared or None


This contradicts what is written above. None type should not raise an exception.

AndreaCrotti · 2016-11-02T12:31:04Z

Just addressed the comments from @divanovGH
Anything else I should do for this @tusharmakkar08 @alecthomas ?

coveralls · 2016-11-02T12:32:41Z

Coverage increased (+0.05%) to 95.968% when pulling 2290b04 on AndreaCrotti:add_maybe_constructor into ad6e78b on alecthomas:master.

tusharmakkar08 · 2016-11-06T18:19:55Z

Hey @AndreaCrotti

Please squash the commits.

Thanks

either of a certain type or None

coveralls · 2016-11-06T21:15:20Z

Coverage increased (+0.05%) to 95.968% when pulling aa9c39d on AndreaCrotti:add_maybe_constructor into caa188e on alecthomas:master.

coveralls · 2016-11-06T21:16:52Z

Coverage increased (+0.05%) to 95.968% when pulling aa9c39d on AndreaCrotti:add_maybe_constructor into caa188e on alecthomas:master.

tusharmakkar08 requested changes Oct 10, 2016

View reviewed changes

AndreaCrotti force-pushed the add_maybe_constructor branch from ca06496 to 0f84edd Compare October 17, 2016 15:56

tusharmakkar08 approved these changes Oct 19, 2016

View reviewed changes

AndreaCrotti force-pushed the add_maybe_constructor branch from 2fa388a to fea2d4d Compare October 21, 2016 10:01

divanovGH reviewed Oct 25, 2016

View reviewed changes

voluptuous/tests/tests.py

s = Schema(Maybe(int))

assert s(1) == 1

Copy link

divanovGH Oct 25, 2016 •

edited

Loading

Choose a reason for hiding this comment

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

Where is your test for: assert s(None) == None ?

divanovGH reviewed Oct 25, 2016

View reviewed changes

AndreaCrotti force-pushed the add_maybe_constructor branch from 282eac3 to 2290b04 Compare November 2, 2016 12:28

AndreaCrotti force-pushed the add_maybe_constructor branch from 2290b04 to 434a25b Compare November 6, 2016 21:12

Add a new validator Maybe(type) which can be used for values that are

aa9c39d

either of a certain type or None

AndreaCrotti force-pushed the add_maybe_constructor branch from 434a25b to aa9c39d Compare November 6, 2016 21:13

tusharmakkar08 merged commit f23056d into alecthomas:master Nov 7, 2016

pyup-bot mentioned this pull request Feb 15, 2018

Update voluptuous to 0.11.1 sohonetlabs/generate-ipdatabase#27

Closed

		@@ -459,6 +459,14 @@ def PathExists(v):
		raise PathInvalid("Not a Path")


		class Maybe(object):

Add a Maybe validator #232

Add a Maybe validator #232

Conversation

AndreaCrotti commented Oct 10, 2016

AndreaCrotti commented Oct 10, 2016

coveralls commented Oct 10, 2016

coveralls commented Oct 10, 2016 • edited Loading

tusharmakkar08 left a comment

Choose a reason for hiding this comment

tusharmakkar08 Oct 10, 2016

Choose a reason for hiding this comment

tusharmakkar08 Oct 10, 2016

Choose a reason for hiding this comment

AndreaCrotti commented Oct 10, 2016

tusharmakkar08 commented Oct 10, 2016

alecthomas commented Oct 10, 2016

tusharmakkar08 commented Oct 10, 2016

AndreaCrotti commented Oct 10, 2016

AndreaCrotti commented Oct 12, 2016

alecthomas commented Oct 17, 2016

coveralls commented Oct 17, 2016 • edited Loading

coveralls commented Oct 17, 2016 • edited Loading

coveralls commented Oct 17, 2016 • edited Loading

coveralls commented Oct 17, 2016

AndreaCrotti commented Oct 18, 2016

coveralls commented Oct 18, 2016 • edited Loading

AndreaCrotti commented Oct 18, 2016

tusharmakkar08 commented Oct 19, 2016

AndreaCrotti commented Oct 19, 2016

AndreaCrotti commented Oct 19, 2016

tusharmakkar08 commented Oct 21, 2016

coveralls commented Oct 21, 2016

coveralls commented Oct 21, 2016 • edited Loading

AndreaCrotti commented Oct 21, 2016

coveralls commented Oct 21, 2016 • edited Loading

AndreaCrotti commented Oct 21, 2016

AndreaCrotti commented Oct 24, 2016

divanovGH Oct 25, 2016 • edited Loading

Choose a reason for hiding this comment

divanovGH Oct 25, 2016

Choose a reason for hiding this comment

AndreaCrotti commented Nov 2, 2016

coveralls commented Nov 2, 2016 • edited Loading

tusharmakkar08 commented Nov 6, 2016

coveralls commented Nov 6, 2016 • edited Loading

coveralls commented Nov 6, 2016 • edited Loading

coveralls commented Oct 10, 2016 •

edited

Loading

coveralls commented Oct 17, 2016 •

edited

Loading

coveralls commented Oct 17, 2016 •

edited

Loading

coveralls commented Oct 17, 2016 •

edited

Loading

coveralls commented Oct 18, 2016 •

edited

Loading

coveralls commented Oct 21, 2016 •

edited

Loading

coveralls commented Oct 21, 2016 •

edited

Loading

divanovGH Oct 25, 2016 •

edited

Loading

coveralls commented Nov 2, 2016 •

edited

Loading

coveralls commented Nov 6, 2016 •

edited

Loading

coveralls commented Nov 6, 2016 •

edited

Loading