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

Fix #772 Empty Only Treated as None #777

Closed
wants to merge 2 commits into from
Closed

Fix #772 Empty Only Treated as None #777

wants to merge 2 commits into from

Conversation

lafrech
Copy link
Member

@lafrech lafrech commented Apr 19, 2018

Implementation of the fix described in #772 (comment).

I also reworded the docstring a bit (this is a rework of #771 which hopefully should close #535). I'm open to suggestions for a better wording.

@deckar01
Copy link
Member

Looks good to me. Thanks @lafrech!

@sloria Should this be released in the next patch of 2.x?

@lafrech
Copy link
Member Author

lafrech commented Apr 19, 2018

I don't know if we have a rule to express the :param types in the docstrings.

Since the text said "tuple or list" I figured I'd use tuple|list as type and remove this from the sentence.

But I might as well be listing all collection types... Maybe tuple was enough and the user can imagine a list will fit as well.

Later in the file, only tuple is specified:

    def validate(self, data, many=None, partial=None):
        """Validate `data` against the schema, returning a dictionary of
        validation errors.
        [...]
        :param bool|tuple partial: Whether to ignore missing fields. If `None`,
            the value for `self.partial` is used. If its value is an iterable,

No big deal anyway. Just trying to get it right.

@sloria
Copy link
Member

sloria commented Apr 24, 2018

Yes, this should be sent to the 2.x-line branch.

Since the text said "tuple or list" I figured I'd use tuple|list as type and remove this from the sentence.

Good idea.

@lafrech Would you mind resending this to 2.x-line?

@sloria
Copy link
Member

sloria commented Apr 26, 2018

Done in #782

@sloria sloria closed this Apr 26, 2018
@lafrech lafrech deleted the dev_772_only_empty branch September 27, 2018 19:00
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.

How is deserialization affected by only and partial?
3 participants