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

Clarify int converter in DEP 201 #43

Closed
wants to merge 1 commit into from
Closed

Clarify int converter in DEP 201 #43

wants to merge 1 commit into from

Conversation

adamchainz
Copy link
Member

I think this should not be 'non-negative' in order to match Python, Flask, and even the example converter that follows in the DEP a few paragraphs later.

@adamchainz
Copy link
Member Author

@sjoerdjob @tomchristie

@sjoerdjob
Copy link
Contributor

In the discussions it has been made clear that int should take positive (or non-negative) input, not arbitrary integers.

This because negative values would most likely be errors, instead of intentional.

(Paraphrasing what Aymeric said in one of the relevant threads which I can't find right now)

@adamchainz
Copy link
Member Author

Ok fair enough, couldn't see any discussion on this either and was confused by the example.

Looked a bit further, Flask is using werkzeug's converters, for which the int one does in fact not accept negative values: https://github.com/pallets/werkzeug/blob/02f5784afce7779450f8f6be7c083dab5670ca68/werkzeug/routing.py#L1032 .

Still, I'd like to suggest the name integer to make it clear this isn't just calling Python's int() (which does accept negative values).

@timgraham
Copy link
Member

This is implemented as currently documented.

@timgraham timgraham closed this Oct 1, 2017
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.

3 participants