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

Difference with re, when repl returns None #457

Open
brondsem opened this issue Mar 9, 2022 · 2 comments
Open

Difference with re, when repl returns None #457

brondsem opened this issue Mar 9, 2022 · 2 comments

Comments

@brondsem
Copy link

brondsem commented Mar 9, 2022

It's not correct for a repl function to return None, but I encountered some code doing that and the stdlib re silently allows it and regex raises an error. Should it be made consistent? Thanks

$ python
Python 3.7.10 (default, Feb 24 2021, 20:00:24)
[GCC 4.8.5 20150623 (Red Hat 4.8.5-36)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import regex, re
>>> def repl(m):
...     return None
...
>>> re.sub(r'a', repl, 'abc')
'bc'
>>> regex.sub(r'a', repl, 'abc')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/var/local/env-py37/lib/python3.7/site-packages/regex/regex.py", line 278, in sub
    return pat.sub(repl, string, count, pos, endpos, concurrent, timeout)
TypeError: expected str instance, NoneType found
@mrabarnett
Copy link
Owner

Well, .findall will never return None for empty groups:

>>> re.search(r'(.)|(.)', 'a').groups()
('a', None)
>>> re.findall(r'(.)|(.)', 'a')
[('a', '')]

For compatibility with re, perhaps regex should do the same, even though it could hide a bug.

I'm a little unsure at this point.

Has re's behaviour ever been reported on the Python bug tracker? If re isn't going to change, if it's intentional behaviour, then I might as well copy it.

@brondsem
Copy link
Author

brondsem commented Mar 9, 2022

I couldn't find an Python bug report for it.

Either way is ok with me, not a huge deal in my situation. Maybe a middle-ground option would be to copy the behavior but issue a Warning?

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

No branches or pull requests

2 participants