Python warning + code cleanup #4981
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LoopedBard3
previously approved these changes
Sep 29, 2025
| '''Return whether the console exporter has been enabled.''' | ||
| return tracing_state_manager.get_console_exporter_enabled() | ||
|
|
||
| # ParamSpec was added in Python 3.10, so we need to use typing_extensions for older versions. |
There was a problem hiding this comment.
How many machines do we have running python <3.10? Maybe we can use this for now but update all machines to latest python and remove it after?
There was a problem hiding this comment.
The only one I found was the alpine queues
There was a problem hiding this comment.
I think we more or less removed everything alpine and no longer run anything on alpine (0b09684) so we may be able to simplify this.
There was a problem hiding this comment.
Pull Request Overview
This PR performs Python code cleanup and warning resolution across the scripts folder, while also updating Python version requirements and improving tooling configurations. The changes standardize type annotations to use Python 3.9+ features and enhance development experience.
- Upgraded minimum Python version requirement from 3.5 to 3.9 and adopted modern built-in type annotations
- Replaced custom argument parser actions with simpler type validators and updated type hints throughout
- Added new MSBuild helper functions and simplified core root building functionality
Reviewed Changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/scenarios/shared/versionmanager.py | Updated type hints to use built-in dict instead of typing.Dict |
| src/scenarios/shared/testtraits.py | Added typing import and updated method parameter type annotation |
| src/scenarios/shared/precommands.py | Replaced typing.List with built-in list type annotations |
| src/scenarios/shared/mauisharedpython.py | Fixed XML attribute access to use direct dictionary syntax |
| src/scenarios/shared/crossgen.py | Updated type annotations for list return types |
| src/scenarios/shared/codefixes.py | Replaced typing.List imports with built-in list annotations |
| scripts/upload.py | Refactored credential handling and added type hints for better error handling |
| scripts/send_to_helix.py | Updated type annotations and replaced custom shell functions with common MSBuild helper |
| scripts/run_performance_job.py | Modernized type hints and replaced manual XML parsing with MSBuild property helper |
| scripts/performance/tracer.py | Improved OpenTelemetry import handling and enhanced type safety |
| scripts/performance/logger.py | Simplified OpenTelemetry logger setup with better error handling |
| scripts/performance/common.py | Added MSBuild helper functions and updated minimum Python version validation |
| scripts/micro_benchmarks.py | Updated function signatures and removed unused functions |
| scripts/dotnet.py | Replaced custom argument actions with type validators and modernized framework handling |
| scripts/ci_setup.py | Updated datetime imports and improved type annotations |
| scripts/channel_map.py | Updated return type annotations for framework and channel methods |
| scripts/build_runtime_payload.py | Added simplified core root building function and fixed tar.gz extraction |
| scripts/benchmarks_monthly_upload.py | Improved authentication flow and error handling |
| scripts/benchmarks_monthly.py | Updated type annotations and removed unnecessary type checks |
| scripts/benchmarks_local.py | Modernized type hints and removed type assertion comments |
| scripts/benchmarks_ci.py | Updated type annotations and simplified framework handling |
| .vscode/settings.json | Enhanced Python analysis with strict mode and better exclusion patterns |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As I was writing a lot of python code for another feature, I was running into lots of python analysis warnings and errors, so I have gone through and resolved all of them in the
scriptsfolder. There are still warnings insrc/scenarioswhich can be tackled separately later if desired.I have also updated the settings.json settings to setup better exclude settings to speed up code searches inside the repository, and enabled workspace-wide python analysis in strict mode so that you can see if any changes affect files you might not have open.
Some other changes in this:
generatelayoutonly, and just does the minimum folder copies. This version is not used anywhere in any pipelines, but is available for use by other tools that want to be able to build coreroots from cached builds to re-run adhoc performance tests.typeinstead ofactionin argparse.