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

Add maxlength minlength examples #509

Closed
wants to merge 2 commits into from

Conversation

vipulgupta2048
Copy link
Contributor

🐣
Added missing examples for maxlength and minlength validation rules to the docs in accordance with #500, let me know if there are any more examples that you like to see. Or improve any examples. Would be happy to write and contribute to the docs.

Signed-off-by: Vipul Gupta (@vipulgupta2048) [email protected]

Copy link
Member

@funkyfuture funkyfuture left a comment

Choose a reason for hiding this comment

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

thanks for your contribution. i made some remarks.

AUTHORS Show resolved Hide resolved
@@ -444,7 +444,8 @@ The assigned data can be of any type.
min, max
--------

Minimum and maximum value allowed for any types that implement comparison operators.
Minimum and maximum value allowed for any types that implement comparison operators. See
Copy link
Member

Choose a reason for hiding this comment

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

proposal: "… implement the corresponding comparison method (__lt__ or __gt__)."

i think a concrete, simple example here would be better than the more complex ones that a referred.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The proposal seems fine, but not sure what are (__lt__ or __gt__), I can just add them if you want. I just felt that by comparison operators we are going for \*of-rules

Roger that, a simple example coming right up.

Copy link
Member

Choose a reason for hiding this comment

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

these "dunder" (from double underscore) or also known as "magic" methods do implement these operations that the operators imply. so, maybe just "comparison operations" instead of "operator"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, @funkyfuture for the explanation.

docs/validation-rules.rst Outdated Show resolved Hide resolved
@@ -457,6 +458,22 @@ minlength, maxlength

Minimum and maximum length allowed for iterables.
Copy link
Member

Choose a reason for hiding this comment

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

on that occassion you could correct one of my mistakes: "iterables" is false in that sentence. truth is that any object that implements __len__ works with this rule. the abstract class is collections.abc.Sized, but i don't think the term "sized" is established and clear.

Copy link
Contributor Author

@vipulgupta2048 vipulgupta2048 Aug 19, 2019

Choose a reason for hiding this comment

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

True that in my project to we had a bit of trouble with collections as we were trying to validate Cerberus Schema and on first thought I just went to validate if the schema is a dict or not. But, later after digging into Mapping and Cerberus's source code that I realized that any Mapping type can be validated. That's a bit of back story I thought I add to the discussion.

Coming back to it, I have changed it now let me know if this works.

@funkyfuture
Copy link
Member

also, could you please change your branch to base on the 1.3.x branch and update the pr accordingly.

@funkyfuture funkyfuture added this to the 1.3.2 milestone Aug 18, 2019
@vipulgupta2048 vipulgupta2048 changed the base branch from master to 1.3.x August 19, 2019 15:23
@vipulgupta2048
Copy link
Contributor Author

Hey @funkyfuture, I guess it's a first time for everything. I have changed the base branch (First time) and implemented the changes. Let me know how you feel about them.
Thanks, ,,

@funkyfuture
Copy link
Member

thanks, merged with 0fccd9b

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.

2 participants