Repository navigation
re.RegexFlag is not included in __all__, makes type inference less useful #75550
Description
| BPO | 31369 |
|---|---|
| Nosy | @gvanrossum, @ezio-melotti, @bitdancer, @ethanfurman, @serhiy-storchaka, @ilevkivskyi, @csabella, @PJB3005, @akulakov |
| PRs |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = <Date 2022-02-05.03:55:29.850>
created_at = <Date 2017-09-06.20:40:02.513>
labels = ['type-bug', 'library', '3.11']
title = 're.RegexFlag is not included in __all__, makes type inference less useful'
updated_at = <Date 2022-02-05.03:55:29.842>
user = 'https://github.com/PJB3005'bugs.python.org fields:
activity = <Date 2022-02-05.03:55:29.842>
actor = 'ethan.furman'
assignee = 'none'
closed = True
closed_date = <Date 2022-02-05.03:55:29.850>
closer = 'ethan.furman'
components = ['Library (Lib)']
creation = <Date 2017-09-06.20:40:02.513>
creator = 'PJB3005'
dependencies = []
files = []
hgrepos = []
issue_num = 31369
keywords = ['patch']
message_count = 12.0
messages = ['301516', '301529', '301532', '301536', '338567', '338583', '338815', '339545', '376815', '376841', '412557', '412558']
nosy_count = 11.0
nosy_names = ['gvanrossum', 'ezio.melotti', 'mrabarnett', 'r.david.murray', 'docs@python', 'ethan.furman', 'serhiy.storchaka', 'levkivskyi', 'cheryl.sabella', 'PJB3005', 'andrei.avk']
pr_nums = ['30279']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue31369'
versions = ['Python 3.11']
Activity
PJB3005 commentedon Sep 6, 2017
It exists and its flags are exported, but not the direct classes itself. This seems inconsistent to me and fixing it would make things like using static typing on it just a little bit easier.
bitdancer commentedon Sep 6, 2017
I think RegexFlag is an implementation detail, but it is true that it isn't prefixed with a _ so putting it in __all__ is not obviously wrong. However, if we do that we should also document it (currently it is mentioned only in a versionchanged line, which isn't technically documenting it).
I find it curious that static typing is affected by __all___, but I don't really know anything about typing.
PJB3005 commentedon Sep 6, 2017
I suppose it may be an implementation detail, though I wouldn't be amazed that had enum existed when re was written it'd have been used instead of constant integers at the time. Though I do suppose exposing it fully would add two ways to get the flags which I can see how it would be considered bad.
It's still useful for type checking though, and while I did make a PR to add it to typeshed, that still leaves it in an iffy state and I probably would not be the last person to be confused by it initially.
bitdancer commentedon Sep 6, 2017
Well, I consider that they really should be named constants and not an enum, which is why I consider it an implementation detail :)
csabella commentedon Mar 21, 2019
@ethan.furman, since you had originally added RegexFlag in bpo-28082, do have an opinion on this? Thanks.
serhiy-storchaka commentedon Mar 22, 2019
I concur with David. This is an imlementation detail. No need to prefix it with a _ if the module uses __all__for public names.
ethanfurman commentedon Mar 25, 2019
I see no reason no prefix
RegexFlagwith an_.As far as adding it to
__all__-- I didn't originally because I was trying to mirror the original implementation, but I am not against it. I would defer that decision to those that work on typing.[-]re.RegexFlag is not included in __all__[/-][+]re.RegexFlag is not included in __all__, makes type inference less useful[/+]ilevkivskyi commentedon Apr 6, 2019
I am totally fine with making RegexFlag public (this may be indeed useful for static typing purposes).
ethanfurman commentedon Sep 13, 2020
Guido, do you have an opinion on adding
RegexFlagto theremodule's__all__and documenting it?There is also some discussion on making these types of int-to-Enum conversions use a
module.namerepr instead ofclass.name:used to be:
>>> re.I <RegexFlag.I: 2>is now:
>>> re.I re.IGNORECASEI of course have no idea if that matters to typing one way or the other.
gvanrossum commentedon Sep 13, 2020
What it prints is irrelevant to static checking.
Currently the typeshed stub for the code already exports RegexFlag, so that the following passes mypy but fails at runtime:
I think it's good to add it to
__all__so this code will not put the type checker to shame, and it would be good to document it.One thing I discovered when developing this example: there doesn't seem to be a flag to represent 0 (zero), i.e. "no flags". And foo(0) is a type error (even though it works fine at runtime).
ethanfurman commentedon Feb 5, 2022
New changeset fea7290 by andrei kulakov in branch 'main':
bpo-31369: include
RegexFlaginre.__all__(GH-30279)fea7290
ethanfurman commentedon Feb 5, 2022
Thanks, everyone!