-
-
Notifications
You must be signed in to change notification settings - Fork 30.8k
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
IDLE is unable to open any .py
files
#104719
Comments
Bisects to 6715f91 (unsurprisingly 😄) |
Will check this today |
Looks like IDLE is using a bunch of non documented attributes of the tokenize module that are no longer used internally. The easier course of action is probably just restore these but note that there will be no guarantee that whatever idle is going with them will continue working. I ran the buildbots and the CI before merge so another question is why CI didn't catch this. |
Do any of the buildbots run the test suite with |
That doesn't seem relevant; |
I checked a bit more how the constant that was eliminated is being used and seems that IDLE is monkey patching the value to modify the behaviour of the now deleted Unfortunately I don't think this is going to work and was out of contract anyway so I think the best course of action here is to eliminate that code from |
FWIW, I can find other places in prominent open-source projects where these undocumented constants (which have now been removed) are used:
So it might be worth adding them back anyway, to limit the disruption caused by this change. |
Most of the projects you mention are using the name of the tokens that are still exposed (as in |
Oh actually I misread, they are using the regular expressions. Hummmm, I still think I prefer to not restore those because they were never part of the public API and I don't want to give any guarantees that they will be up to date and doing whatever they were doing before. |
To be clear, I fully agree that these projects should not have been using undocumented constants from the |
Yes, to be clear:
|
Probably this is what we will end up doing because it makes sense to reduce breakage in general but I am kind of torn here because I don't want to normalise adding deprecation warnings for APIs and constants that were never public and also it makes me a bit uncomfortable because for instance even if we add back Furthermore, as these constants were not public interface they have no tests or any way to validate that they work and that nobody breaks them in the future by accident and this makes me very uncomfortable giving any sort of guarantee even if they are informal. @ |
CC: @Yhg1s thoughts? |
Yeah, I agree it doesn't make much sense to add (Here's all the names that aren't in
|
Searching the top 5k PyPI projects (for direct uses only, like Found 28 matching lines in 7 projects❯ python3 ~/github/misc/cpython/search_pypi_top.py -q . "tokenize\.(any|Binnumber|Comment|ContStr|Decnumber|Double|Double3|endpats|Expfloat|Exponent|Floatnumber|Funny|group|Hexnumber|Ignore|Imagnumber|Intnumber|maybe|Name|Number|Octnumber|PlainToken|Pointfloat|PseudoExtras|PseudoToken|Single|single_quoted|Single3|Special|String|StringPrefix|tabsize|Token|Triple|triple_quoted|Whitespace)\b"
./vowpalwabbit-9.8.0.tar.gz: vowpalwabbit-9.8.0/ext_libs/rapidjson/thirdparty/gtest/googlemock/scripts/generator/cpp/ast.py: type_name.append(tokenize.Token(tokenize.SYNTAX, ' ', 0, 0))
./vowpalwabbit-9.8.0.tar.gz: vowpalwabbit-9.8.0/ext_libs/rapidjson/thirdparty/gtest/googlemock/scripts/generator/cpp/ast.py: last_token = tokenize.Token(tokenize.SYNTAX, ';', 0, 0)
./vowpalwabbit-9.8.0.tar.gz: vowpalwabbit-9.8.0/ext_libs/rapidjson/thirdparty/gtest/googlemock/scripts/generator/cpp/ast.py: name = tokenize.Token(tokenize.NAME, 'operator[]',
./vowpalwabbit-9.8.0.tar.gz: vowpalwabbit-9.8.0/ext_libs/rapidjson/thirdparty/gtest/googlemock/scripts/generator/cpp/ast.py: seq_copy.append(tokenize.Token(tokenize.SYNTAX, '', 0, 0))
./vowpalwabbit-9.8.0.tar.gz: vowpalwabbit-9.8.0/ext_libs/rapidjson/thirdparty/gtest/googlemock/scripts/generator/cpp/ast.py: struct = tokenize.Token(tokenize.NAME, 'struct',
./vowpalwabbit-9.8.0.tar.gz: vowpalwabbit-9.8.0/ext_libs/rapidjson/thirdparty/gtest/googlemock/scripts/generator/cpp/ast.py: if tokens and isinstance(tokens[0], tokenize.Token):
./vowpalwabbit-9.8.0.tar.gz: vowpalwabbit-9.8.0/ext_libs/rapidjson/thirdparty/gtest/googlemock/scripts/generator/cpp/ast.py: internal_token = tokenize.Token(_INTERNAL_TOKEN, _NAMESPACE_POP,
./onnxsim-0.4.26.tar.gz: onnxsim-0.4.26/third_party/onnx-optimizer/third_party/protobuf/third_party/googletest/googlemock/scripts/generator/cpp/ast.py: type_name.append(tokenize.Token(tokenize.SYNTAX, ' ', 0, 0))
./onnxsim-0.4.26.tar.gz: onnxsim-0.4.26/third_party/onnx-optimizer/third_party/protobuf/third_party/googletest/googlemock/scripts/generator/cpp/ast.py: last_token = tokenize.Token(tokenize.SYNTAX, ';', 0, 0)
./onnxsim-0.4.26.tar.gz: onnxsim-0.4.26/third_party/onnx-optimizer/third_party/protobuf/third_party/googletest/googlemock/scripts/generator/cpp/ast.py: name = tokenize.Token(tokenize.NAME, 'operator[]',
./onnxsim-0.4.26.tar.gz: onnxsim-0.4.26/third_party/onnx-optimizer/third_party/protobuf/third_party/googletest/googlemock/scripts/generator/cpp/ast.py: seq_copy.append(tokenize.Token(tokenize.SYNTAX, '', 0, 0))
./onnxsim-0.4.26.tar.gz: onnxsim-0.4.26/third_party/onnx-optimizer/third_party/protobuf/third_party/googletest/googlemock/scripts/generator/cpp/ast.py: struct = tokenize.Token(tokenize.NAME, 'struct',
./onnxsim-0.4.26.tar.gz: onnxsim-0.4.26/third_party/onnx-optimizer/third_party/protobuf/third_party/googletest/googlemock/scripts/generator/cpp/ast.py: if tokens and isinstance(tokens[0], tokenize.Token):
./onnxsim-0.4.26.tar.gz: onnxsim-0.4.26/third_party/onnx-optimizer/third_party/protobuf/third_party/googletest/googlemock/scripts/generator/cpp/ast.py: internal_token = tokenize.Token(_INTERNAL_TOKEN, _NAMESPACE_POP,
./onnxoptimizer-0.3.13.tar.gz: onnxoptimizer-0.3.13/third_party/protobuf/third_party/googletest/googlemock/scripts/generator/cpp/ast.py: type_name.append(tokenize.Token(tokenize.SYNTAX, ' ', 0, 0))
./onnxoptimizer-0.3.13.tar.gz: onnxoptimizer-0.3.13/third_party/protobuf/third_party/googletest/googlemock/scripts/generator/cpp/ast.py: last_token = tokenize.Token(tokenize.SYNTAX, ';', 0, 0)
./onnxoptimizer-0.3.13.tar.gz: onnxoptimizer-0.3.13/third_party/protobuf/third_party/googletest/googlemock/scripts/generator/cpp/ast.py: name = tokenize.Token(tokenize.NAME, 'operator[]',
./onnxoptimizer-0.3.13.tar.gz: onnxoptimizer-0.3.13/third_party/protobuf/third_party/googletest/googlemock/scripts/generator/cpp/ast.py: seq_copy.append(tokenize.Token(tokenize.SYNTAX, '', 0, 0))
./onnxoptimizer-0.3.13.tar.gz: onnxoptimizer-0.3.13/third_party/protobuf/third_party/googletest/googlemock/scripts/generator/cpp/ast.py: struct = tokenize.Token(tokenize.NAME, 'struct',
./onnxoptimizer-0.3.13.tar.gz: onnxoptimizer-0.3.13/third_party/protobuf/third_party/googletest/googlemock/scripts/generator/cpp/ast.py: if tokens and isinstance(tokens[0], tokenize.Token):
./onnxoptimizer-0.3.13.tar.gz: onnxoptimizer-0.3.13/third_party/protobuf/third_party/googletest/googlemock/scripts/generator/cpp/ast.py: internal_token = tokenize.Token(_INTERNAL_TOKEN, _NAMESPACE_POP,
./os_sys-2.1.4.tar.gz: os_sys-2.1.4/edit/editor.py: save_tabsize = tokenize.tabsize
./os_sys-2.1.4.tar.gz: os_sys-2.1.4/edit/editor.py: tokenize.tabsize = self.tabwidth
./os_sys-2.1.4.tar.gz: os_sys-2.1.4/edit/editor.py: tokenize.tabsize = save_tabsize
./pandas-2.0.1.tar.gz: pandas-2.0.1/pandas/_config/config.py: if not re.match("^" + tokenize.Name + "$", k):
./recordlinkage-0.15.tar.gz: recordlinkage-0.15/recordlinkage/config.py: if not bool(re.match('^' + tokenize.Name + '$', k)):
./libcst-0.4.9.tar.gz: libcst-0.4.9/libcst/_parser/base_parser.py: # - Supports our custom Token class, instead of `parso.python.tokenize.Token`.
./libcst-0.4.9.tar.gz: libcst-0.4.9/libcst/_parser/types/token.py: Token = tokenize.Token
Time: 0:00:22.471982
Found 28 matching lines in 7 projects |
@terryjreedy, I think the topic-IDLE label makes sense here, no? The potential release blocker, to my mind, is that IDLE is broken on |
The 101 lines, lines 59 to 159, were deleted. These are all PUBLIC names with no indication in the module itself that they are private in any sense. Reading the corresponding doc is optional. To me, removing them without deprecation is a gross violation of back compatibility rules. I am investigating the IDLE issue. |
They are not documented in the docs nor exposed in
I disagree (but I understand your position): the docs and the In any case, as I mention, I think the correct thing here is restore all these constants because it doesn't give us a lot of maintainance burden and minimizes breakage, so is an easy fix. @mgmacias95 will make a PR |
…nGH-104726) (cherry picked from commit 0c5e79b) Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
Reopening, as there's still two things left to do here:
|
This class contains all editor references to tokenize module.
I decided that it would in general be a good idea to have test coverage of references to non-idlelib modules. I have a toktest branch that adds coverage of the class containing the failure and all tokenize references in editor.py. I will make a PR tomorrow with or without coverage of tokenize references in 2 other idlelib modules. |
FWIW, whether to deprecate or document the unused globals isn't really a RM question, but considering the difficulty of emitting warnings for them, I'd rather not do that right now (but for 3.13 it's fine). Documenting them as for internal use only and/or deprecated can be done after b1. |
Class editor.IndentSearcher contains all editor references to tokenize module. Module io tokenize reference cover those other modules. Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
…ythonGH-104767) Class editor.IndentSearcher contains all editor references to tokenize module. Module io tokenize reference cover those other modules. (cherry picked from commit e561c09) Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu> Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
…ythonGH-104767) Class editor.IndentSearcher contains all editor references to tokenize module. Module io tokenize reference cover those other modules. (cherry picked from commit e561c09) Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu> Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
…H-104767) (#104844) gh-104719: IDLE - test existence of all tokenize references. (GH-104767) Class editor.IndentSearcher contains all editor references to tokenize module. Module io tokenize reference cover those other modules. (cherry picked from commit e561c09) Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu> Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
…H-104767) (#104845) gh-104719: IDLE - test existence of all tokenize references. (GH-104767) Class editor.IndentSearcher contains all editor references to tokenize module. Module io tokenize reference cover those other modules. (cherry picked from commit e561c09) Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu> Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
IDLE now has a regression test that will fail if any tokenizer names change. I suggest we close this and someone open a new issue about the now obsolete constants. |
That sounds good to me. I'll try to open an issue later today. Thanks @terryjreedy, @pablogsal and @mgmacias95! |
With a fresh CPython build (be0c106), IDLE is unable to open any
.py
files.To reproduce:
.py
file with the namerepro.py
python -m idlelib repro.py
IDLE still seems able to create new
.py
files and save them; it just can't open pre-existing.py
files right now.Traceback observed
Environment
(Given the cause of the bug, the environment details shouldn't really be relevant; but they're included here anyway, for completeness.)
Reproduces on a debug and non-debug build, FWIW.
Linked PRs
The text was updated successfully, but these errors were encountered: