Skip to content

Add enum parser#185

Merged
sloria merged 7 commits intosloria:masterfrom
tomgrin10:enum-parser
Nov 7, 2020
Merged

Add enum parser#185
sloria merged 7 commits intosloria:masterfrom
tomgrin10:enum-parser

Conversation

@tomgrin10
Copy link
Copy Markdown
Contributor

The enum parser receives the enum type and parses the enum by its name.

Copy link
Copy Markdown
Owner

@sloria sloria left a comment

Choose a reason for hiding this comment

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

Thanks! This looks good. Just some nitpicking to address.

Comment thread environs/__init__.py Outdated
Comment thread environs/__init__.py Outdated
@tomgrin10
Copy link
Copy Markdown
Contributor Author

Applied the KeyError suggestion and added a bonus ignore_case argument :)

Copy link
Copy Markdown
Owner

@sloria sloria left a comment

Choose a reason for hiding this comment

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

Just a few more tiny changes

Comment thread .gitignore Outdated
Comment thread environs/__init__.py Outdated
Comment thread tests/test_environs.py
@tomgrin10
Copy link
Copy Markdown
Contributor Author

Updated the readme, and left a comment on the from error nitpick

Copy link
Copy Markdown
Owner

@sloria sloria left a comment

Choose a reason for hiding this comment

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

Looks good! I'll merge and release this soon when I'm off the clock

Comment thread environs/__init__.py Outdated
Comment thread README.md
@env.parser_for("enum")
def enum_parser(value, choices):
@env.parser_for("choice")
def choice_parser(value, choices):
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

thought (non-blocking): We might want to come up with a different example here since users would use env.enum for this use case rather than a custom parser.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think the example isn't that bad for now, this deserves a separate issue anyway.

@tomgrin10 tomgrin10 closed this Nov 4, 2020
@tomgrin10 tomgrin10 reopened this Nov 4, 2020
@sloria sloria merged commit 237ebdf into sloria:master Nov 7, 2020
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