Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 33.9k
gh-108303: Move tokenize-related data to Lib/test/tokenizedata#109265
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
gh-108303: Move tokenize-related data to Lib/test/tokenizedata#109265
Uh oh!
There was an error while loading. Please reload this page.
Conversation
sobolevn commented Sep 11, 2023 • edited by bedevere-app bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-app bot
Uh oh!
There was an error while loading. Please reload this page.
serhiy-storchaka commented Sep 11, 2023
Should not |
vstinner commented Sep 11, 2023
I think so, yes. |
Lib/test/test_tokenize.py Outdated
| fn=support.findfile("tokenize_tests.txt") | ||
| tempdir=os.path.dirname(fn) oros.curdir | ||
| fn=support.findfile("tokenize_tests.txt", subdir='tokenizedata') | ||
| tempdir=os.path.dirname(os.path.dirname(fn)) oros.curdir |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about the logical to get tempdir here. Maybe start from file instead? Why is it called tempdir? That's Lib/test/, the directory of test files, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not something I am modifing here. I only changedtempdir = os.path.dirname(fn) or os.curdir
to betempdir = os.path.dirname(os.path.dirname(fn)) or os.curdir
I can open a new issue about it and dig into it later :)
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Victor Stinner <vstinner@python.org>
vstinner left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
vstinner commented Sep 11, 2023
@serhiy-storchaka: Do you want to review the updated PR? It LGTM. |
sobolevn commented Sep 11, 2023
@vstinner thanks for your help! 👍 |
Uh oh!
There was an error while loading. Please reload this page.
serhiy-storchaka commented Sep 12, 2023
Should it be backported? |
sobolevn commented Sep 12, 2023
bedevere-bot commented Sep 12, 2023
|
bedevere-bot commented Sep 12, 2023
|
vstinner commented Sep 12, 2023
I don't think so. I prefer to minimize backports, and here it's unlikely that we modify test files in stable versions. |
vstinner commented Sep 12, 2023
@sobolevn: Oh, you need to add the new directory to TESTSUBDIRS in Makefile.pre.in. Can you please create a new PR to fix that? |
bedevere-bot commented Sep 12, 2023
|
vstinner commented Sep 12, 2023
…ythonGH-109265) (cherry picked from commit 1110c5b)
GH-109677 is a backport of this pull request to the 3.12 branch. |
…ythonGH-109265) (cherry picked from commit 1110c5b)
GH-109678 is a backport of this pull request to the 3.11 branch. |
In the future, I think, we can move more files to this folder, since there are quite a lot of artefacts for
tokenizeand similar tools.