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 attr.validators.in_() #181

Merged
merged 5 commits into from May 16, 2017

Conversation

Projects
None yet
3 participants
@dieb
Contributor

dieb commented May 1, 2017

Hello there!

I wrote this for one of my projects and I thought it could be useful to others too. This is a validator called in_(options) that allows you to check if the value is within the allowed values. In my particular case it was useful to test things like in_(POSSIBLE_CHOICES), in_(Animals), etc.

Example:

US_STATES = ['AB', 'AG', 'AK', ...]

@attr(slots=True)
class Location(object):
    city = attr.ib(validator=instance_of(str))
    state = attr.ib(validator=in_(US_STATES))

I was wondering if you had any thoughts and whether it would make sense to add it to attrs. If that's the case, I'd be more than happy to adjust the code with your suggestions.

Thanks!

Pull Request Check List

This is just a reminder about the most common mistakes. Please make sure that you tick all appropriate boxes. But please read our contribution guide at least once, it will save you unnecessary review cycles!

  • Added tests for changed code.
  • New features have been added to our Hypothesis testing strategy.
  • Updated documentation for changed code.
  • Documentation in .rst files is written using semantic newlines.
  • Changed/added classes/methods/functions have appropriate versionadded, versionchanged, or deprecated directives.
  • Changes (and possible deprecations) are documented in CHANGELOG.rst.

If you have any questions to any of the points above, just submit and ask! This checklist is here to help you, not to deter you from contributing!

@codecov

This comment has been minimized.

codecov bot commented May 1, 2017

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #181   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           9      9           
  Lines         579    589   +10     
  Branches      126    128    +2     
=====================================
+ Hits          579    589   +10
Impacted Files Coverage Δ
src/attr/validators.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 a328e67...cc5499d. Read the comment docs.

@hynek

This comment has been minimized.

Member

hynek commented May 4, 2017

This seems like a good idea however the naming (and docstring) seems too restrictive given the implementation. Something like is_in(possible_values) seems more natural to me?

@dieb

This comment has been minimized.

Contributor

dieb commented May 4, 2017

is_in sounds good. Given that other validators are single words (e.g. optional), what do you think about within?

@hynek

This comment has been minimized.

Member

hynek commented May 10, 2017

@glyph try to talk me out of calling it in_() please. We have precedents in sqlchemistry. ;)

@glyph

This comment has been minimized.

Contributor

glyph commented May 10, 2017

found_in?

@hynek

This comment has been minimized.

Member

hynek commented May 12, 2017

Since we’re going for and_() in #186 and ppl are used to them from sqlalchemy, please go for in_().

@hynek

This comment has been minimized.

Member

hynek commented May 12, 2017

(I cannot promise this will make it into 17.1 b/c I need to get it out ASAP but it’s possible. If you want to raise your chances, please double check CONTRIBUTING.rst; especially re: test docstrings.)

@dieb

This comment has been minimized.

Contributor

dieb commented May 12, 2017

Just updated it to in_().

@hynek

hynek approved these changes May 16, 2017

@hynek hynek merged commit 461e1b2 into python-attrs:master May 16, 2017

3 checks passed

codecov/patch 100% of diff hit (target 100%)
Details
codecov/project 100% (+0%) compared to a328e67
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@hynek

This comment has been minimized.

Member

hynek commented May 16, 2017

Thanks, I’ve made some small adjustments.

@dieb dieb deleted the dieb:enhancement/one-of-validator branch May 16, 2017

@dieb

This comment has been minimized.

Contributor

dieb commented May 16, 2017

Thanks!

@dieb dieb changed the title from [RFC] Enhancement/one of validator to Add attr.validators.in_() May 16, 2017

@dieb

This comment has been minimized.

Contributor

dieb commented May 16, 2017

I just noticed I forgot to add in_() to __all__, I'll submit a PR later on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment