| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
Hello!
|
Sorry, something went wrong.
Thanks for the quick feedback. I've committed a first pass of 1-3. As for tests: there appear to be no importtime tests (likely due to the fact that any simple implementation would be super noisy as other parts of the interpreter change). Would it be sufficient to do some rudimentary check i.e. import foo then ensure a corresponding foo row appears in the output? Thanks again! |
Sorry, something went wrong.
Yes, it would be sufficient :) |
Sorry, something went wrong.
|
Also, there's a merge conflict. Can you resolve it? (I can do it myself, but it seems like you have turned off this option) |
Sorry, something went wrong.
Done, just added tests as well |
Sorry, something went wrong.
|
@Eclips4 who should I ping to get final review? Is it alright if I @ the blamed reviewers? |
Sorry, something went wrong.
I guess @vstinner is the right person to review this 😄 |
Sorry, something went wrong.
The same applies for the versionchanged directives. |
Sorry, something went wrong.
|
All checks except for mypy should pass. The issue reported by mypy is the following idiom try:
import posix
except ImportError:
posix = NoneSince this is used pretty widely throughout the standard library, I figured it should be left as is and the warning ignored. I could suppress the warning instead. Update: woah, I lied, something has changed since my last push. I'll fix these failures today. |
Sorry, something went wrong.
You can mypy-ignore it I think. Otherwise, future modifications in that file would also be flagged, right? |
Sorry, something went wrong.
Sorry, something went wrong.
|
@AA-Turner merged your changes, happy to do anything else necessary, apologies for the inconvenience of the organization branch. Not sure where your comment went (I saw it in my email), but thanks for helping push this through. |
Sorry, something went wrong.
|
Thanks @noahbkim, no worries! Are you able to make the changes to config_set_import_time etc to reserve values >=2? We also recently changed our CLA bot, please can you click on the red 'not signed' and ensure everything is up to date? A |
Sorry, something went wrong.
|
I think I've messed up my branch, I'm going to attempt to squash everything so I can overwrite my commit email correctly. |
Sorry, something went wrong.
|
Ok, please ping for a review when you've sorted the branch out! If it's easier, feel free to just open a new PR, but a force push here should also work. A |
Sorry, something went wrong.
|
@AA-Turner I've squashed my changes and given everything a second pass. It looks like I caused some kind of regression in test_embed.py, but none of the expected invariants make sense to me (I'm not sure why we expect import_time to be set to 1 in any of the listed circumstances). Looking through the blame was not particularly illuminating. My suspicion is that these tests have been migrated/updated several times without regard for what they're actually testing (or perhaps implicit behavior surrounding -Ximporttime has changed since the test suite's addition?). Zeroing import_time in said cases returns everything to passing, but a second pair of eyes on that particular bit would be greatly appreciated. I have about 6 tests failing locally, one of which prevents me from compiling with --enable-optimizations (test_json) but I can't imagine those are related. I'll check if my merge base is behind + fix the CI lint issue. |
Sorry, something went wrong.
| import nt | ||
| return nt._supports_virtual_terminal() | ||
| except (ImportError, AttributeError): | ||
| nt._supports_virtual_terminal() |
There was a problem hiding this comment.
| nt._supports_virtual_terminal() | |
| return nt._supports_virtual_terminal() |
This is missing a return and would never enable virtual terminal anymore.
Sorry, something went wrong.
|
@noahbkim I've pushed a series of commits to the 3.14-importtime=2 branch on my fork. I've created a PR to HRT's fork (hudson-trading#1), which if merged will automatically update this PR with those commits. Please review them and let me know what you think. In short:
When these changes are addressed, we can run the buildbots on this PR and check that everything looks alright. Thanks! A |
Sorry, something went wrong.
|
ping @noahbkim (feature freeze is tomorrow) |
Sorry, something went wrong.
`-X importtime=2` feedback (Thanks so much, @AA-Turner)
|
@AA-Turner I am incredibly grateful for the work you've done to get this over the finish line. Thanks again, I'll have an eye on Github/my email all day in case you need anything else. |
Sorry, something went wrong.
|
🤖 New build scheduled with the buildbot fleet by @AA-Turner for commit 6d50b0c 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F118655%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
Sorry, something went wrong.
|
Buildbots show leaks in test_datetime which seem unrelated. |
Sorry, something went wrong.
There was a problem hiding this comment.
Thanks!
A
Sorry, something went wrong.
| -X importtime[=2]: show how long each import takes; use -X importtime=2 to\ | ||
| log imports of already-loaded modules; also PYTHONPROFILEIMPORTTIME\n\ |
There was a problem hiding this comment.
This is missing a newline:
| -X importtime[=2]: show how long each import takes; use -X importtime=2 to\ | |
| log imports of already-loaded modules; also PYTHONPROFILEIMPORTTIME\n\ | |
| -X importtime[=2]: show how long each import takes; use -X importtime=2 to\n\ | |
| log imports of already-loaded modules; also PYTHONPROFILEIMPORTTIME\n\ |
$ python3.14 --help-all
...
-X frozen_modules=[on|off]: whether to use frozen modules; the default is "on"
for installed Python and "off" for a local build;
also PYTHON_FROZEN_MODULES
-X importtime[=2]: show how long each import takes; use -X importtime=2 to log imports of already-loaded modules; also PYTHONPROFILEIMPORTTIME
-X int_max_str_digits=N: limit the size of int<->str conversions;
0 disables the limit; also PYTHONINTMAXSTRDIGITS
-X no_debug_ranges: don't include extra location information in code objects;
also PYTHONNODEBUGRANGES
-X perf: support the Linux "perf" profiler; also PYTHONPERFSUPPORT=1
...Please see PR #136391 for a fix.
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
As mentioned in the issue:
The updated example (-Ximporttime=2):
With -Ximporttime:
Discussion: https://discuss.python.org/t/x-importtrace-to-supplement-x-importtime-for-loaded-modules/23882/5
Prior email chain: https://mail.python.org/archives/list/python-ideas@python.org/thread/GEISYQ5BXWGKT33RWF77EOSOMMMFUBUS/