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

[Python] Wrong input for python regex #3610

Closed
Freymaurer opened this issue Nov 24, 2023 · 9 comments · Fixed by #3627
Closed

[Python] Wrong input for python regex #3610

Freymaurer opened this issue Nov 24, 2023 · 9 comments · Fixed by #3627
Labels
python Python

Comments

@Freymaurer
Copy link

Using Regex.Match(..,..) in dotnet, it gets transpiled to the following in fable_library/reg_exp.py:

def match(reg: Pattern[str] | str, input: str, start_at: int = 0) -> Match[str] | None:
    if isinstance(reg, str):
        flags = _options_to_flags(start_at)
        return re.search(input, reg, flags)
    return reg.search(input, pos=start_at)

But re.search get its arguments in the order of: pattern, input... . So pattern and input are switched, which crashes obviously all matches.

image (1)

😅

@Freymaurer
Copy link
Author

Okay, that was not the error, as the match() function also has their arguments switched. So two false make a positive in this case. Propably best to update the argument naming.

@MangelMaxime MangelMaxime added the python Python label Nov 24, 2023
@MangelMaxime MangelMaxime changed the title [FABLE PYTHON] Wrong input for python regex [Python] Wrong input for python regex Nov 24, 2023
@dbrattli
Copy link
Collaborator

dbrattli commented Nov 28, 2023

@Freymaurer What F# code would crash on all matches? I'm not sure I understand how you trigged an error here. The code is obviously wrong, but I don't think it's ever used, ref: 7f494c4#diff-d143e99366f0ba295fb8bbc80bca86f86884323796fb2da0e473e8a8dcf7eeb0

@Freymaurer
Copy link
Author

Hey! The issue actually was us not checking for python regex dialect.

But when we lloked at the source code in python we saw that "input und pattern" are switched between re.search and the match function. Thats why we thought there is a bug in the source code. But the match function was also given input and pattern switched so "two false make a right" everything is working correctly, just the function arguments have the wrong names.

@Freymaurer
Copy link
Author

I hope i was able to explain the issue @dbrattli 😅 Everything works as intended, but the argument names of match are wrong

@dbrattli
Copy link
Collaborator

Are you sure? How do you see they are wrong? I tried:

let regex = Regex("pattern")
let mat = regex.Match("input")

This transpiles to:

regex: Any = create("pattern")
mat: Any = match(regex, "input")

So the pattern is the first argument and input text is the second which seems right?

@Freymaurer
Copy link
Author

Have a look at the image from my original post, there it says:

return re.search(input, reg, flags)

With reg being defined as Pattern in the match function. But intellisense in re.search expects the arguments in the order of:

Pattern -> string -> FlagsType

But according to the match arguments we give it in the order of:

string -> Pattern -> FlagsType

@dbrattli
Copy link
Collaborator

Yes, my question was how did you trigger that part of the code to run? I'm not sure it's even used since it's been removed in the TypeScript specific part of Fable. There looks to be no unit-test that hits that code. So just want to check with you how/if you manged to call that code-line. If not, I will just remove the code.

@Freymaurer
Copy link
Author

A sorry i must have misunderstood!

Here is a REPL of a minimal example.

@dbrattli
Copy link
Collaborator

Thanks, now I see the problem. Making a fix for this ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python Python
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants