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

The float converter does not recognise whole numbers #1645

Closed
logi opened this issue Sep 18, 2019 · 3 comments
Closed

The float converter does not recognise whole numbers #1645

logi opened this issue Sep 18, 2019 · 3 comments

Comments

@logi
Copy link

logi commented Sep 18, 2019

The float converter does not allow integers (123) or floats with trailing periods (123.) but either of those are likely to be created by both machine and human clients of an API when they are told to enter a number or even a float.

This is also causing our APIs created with Connexion from an OpenApi spec to break that spec since OpenApi does not require values of the number type to have a period followed by a number and the routing is delegated to Flask. See spec-first/connexion#1041

I propose a solution similar to #729, adding an allow_int argument to the float converter which would relax the regular expression and is False by default. I'm willing to make that pull request if I think it might in principle be merged.

@carc1n0gen
Copy link

For accepting an integer, just add another route decorator that is the same except with the int converter. I don't have a solution to the other though

@davidism
Copy link
Member

It's deliberate that the converters accept one representation for each type. If you need something else, use a custom converter:

from werkzeug.routing import BaseConverter

class IntFloatConverter(BaseConverter):
    regex = r"-?(?:\d+(?:\.(?:\d+)?)?|\.\d+)"

    def to_python(self, value):
        try:
            return int(value)
        except ValueError:
            return float(value)

app.url_map.converters["number"] = IntFloatConverter

@logi
Copy link
Author

logi commented Sep 18, 2019

Adding another route accepting an int for each and every float is tedious and error prone and once you have four floats in your path (for us it's a bounding box with two lat,lon pairs) you're up to 16 routes.

Yes, I saw the "one representation" line before, but that is neither achievable nor desirable. These are a few different representations of the same number:

  • 1
  • 1.
  • 1.0
  • 1.00
  • 1.00000000
  • 00001.0
  • 0.99999999999999999

What exactly is gained by excluding the first two in that list? The downside is obvious in that we now have to explain to all clients that they have to make sure that whatever they use to format the arguments into a URL uses a pattern that forces at least one digit after the decimal point (and you'd better unit test that). We can't even make a good error message since it'll just come out as 404 if you pass in your numbers without a superfluous trailing .0. So that's how you end up with numbers parsed in as strings and ad-hoc regular expressions checking them, which is being recommended all over the net.

@davidism yes, I guess making a number converter and an integer converter is at least much easier for me. And it would allow me to fix our particular issue in just the connexion code-base.

@logi logi closed this as completed Sep 18, 2019
@logi logi reopened this Sep 18, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants