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

is_in_bounds parses majorant incorrectly when bounding range is floats in string form #140

Open
bandophahita opened this issue Apr 4, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@bandophahita
Copy link
Contributor

is_in_bounds("[1.0, 4.0]") should return a majorant of 4.0 but instead returns 0.0

i1 = is_in_bounds("(1.0, 4.0)")
print(i1)

the number is within the range of 1.0 and 0.0

@bandophahita bandophahita added the bug Something isn't working label Apr 4, 2024
@bandophahita bandophahita changed the title is_in_bounds parse majorant when float in string form is_in_bounds parses majorant incorrectly when bounding range is floats in string form Apr 4, 2024
@bandophahita
Copy link
Contributor Author

I see the problem but my regex-fu is weak. We need a regex that can find int or float.

@perrygoy
Copy link
Member

perrygoy commented Apr 4, 2024

The pattern is currently:

        pattern = (
            r"^(?P<lower>[\[\(]?)"
            r"(?P<minorant>\d+).*?(?P<majorant>\d+)"
            r"(?P<upper>[\]\)]?)$"
        )

It should be:

        pattern = (
            r"^(?P<lower>[\[\(]?)"
            r"(?P<minorant>\d+\.?\d*).*?(?P<majorant>\d+\.?\d*)"
            r"(?P<upper>[\]\)]?)$"
        )

This regex should match a single . if it's present, which would raise for no match if someone accidentally puts an IP address in there or something. It doesn't match a . and then expect at least one number afterwards, but it looks like Python doesn't care if they just put a period:

>>> float("4.")
4.0

@bandophahita
Copy link
Contributor Author

bandophahita commented Apr 4, 2024

We should also cover the case where users submit .5. i.e. `is_in_bounds("[.5, .9]")

I found [.\d]+ works and we can let the cast to float deal with the possibility of something like IP addresses.

        pattern = (
            r"^(?P<lower>[\[\(]?)"
            r"(?P<minorant>[.\d]+).*?(?P<majorant>[.\d]+)"
            r"(?P<upper>[\]\)]?)$"
        )
    def test_bad_floats(self) -> None:
        with pytest.raises(
            ValueError, match="could not convert string to float: '1.1.1.1'"
        ):
            is_in_bounds("(1.1.1.1, 4.0]")

@perrygoy
Copy link
Member

perrygoy commented Apr 4, 2024

Of course .\d works because . matches any character. Use the pattern in my previous comment please!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants