Overhauled CLI.#963
Conversation
| """Handle the 'view' subcommand to view an existing profile.""" | ||
| import json | ||
| import subprocess | ||
| import scalene.scalene_config |
Check notice
Code scanning / CodeQL
Module is imported with 'import' and 'import from' Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 months ago
To fix the problem, we should remove the redundant import scalene.scalene_config statement inside the _handle_view_command method. Instead, we should import SCALENE_PORT alongside the other named imports from scalene.scalene_config at the top of the file, since this variable is the only member being referenced from the module after line 692. Then, within the code, change any access of scalene.scalene_config.SCALENE_PORT to just SCALENE_PORT. This preserves functionality and increases clarity, while resolving the potential for confusion and the warning generated by CodeQL.
Specifically:
- Edit the top-level import (
from scalene.scalene_config import scalene_date, scalene_version) to also importSCALENE_PORT. - Remove line 692:
import scalene.scalene_config - Update line 737: change
scalene.scalene_config.SCALENE_PORTto justSCALENE_PORT.
No other changes are needed.
| @@ -11,7 +11,7 @@ | ||
|
|
||
| from scalene.find_browser import find_browser | ||
| from scalene.scalene_arguments import ScaleneArguments | ||
| from scalene.scalene_config import scalene_date, scalene_version | ||
| from scalene.scalene_config import scalene_date, scalene_version, SCALENE_PORT | ||
| from scalene.scalene_statistics import Filename | ||
| from scalene.scalene_utility import generate_html | ||
|
|
||
| @@ -689,7 +689,6 @@ | ||
| import json | ||
| import subprocess | ||
|
|
||
| import scalene.scalene_config | ||
|
|
||
| profile_file = args.profile_file | ||
| output_file = "scalene-profile.html" | ||
| @@ -734,7 +733,7 @@ | ||
| sys.executable, | ||
| f"{dir}{os.sep}launchbrowser.py", | ||
| os.path.abspath(output_file), | ||
| str(scalene.scalene_config.SCALENE_PORT), | ||
| str(SCALENE_PORT), | ||
| ], | ||
| stdout=subprocess.DEVNULL, | ||
| stderr=subprocess.DEVNULL, |
| def _parse_args_impl( | ||
| defaults: ScaleneArguments, | ||
| ) -> Tuple[argparse.Namespace, List[str]]: |
Check notice
Code scanning / CodeQL
Explicit returns mixed with implicit (fall through) returns Note
| user_site = site.getusersitepackages() | ||
| if user_site: | ||
| paths.add(os.path.normpath(user_site)) | ||
| except Exception: |
Check notice
Code scanning / CodeQL
Empty except Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 months ago
The best practice for fixing this issue is to avoid silently suppressing all exceptions. Instead, catch the exceptions, log an informative warning, and only suppress non-critical (non-fatal) errors. This can be done by calling warnings.warn() (already imported in the file) within the except block and including the exception message/context in the warning. The placement of the warning should make it clear this is related to site-packages discovery. No additional imports or method definitions are needed since warnings is already available.
Lines to change:
In scalene/scalene_profiler.py, lines 1165-1166 should be modified to use warnings.warn to report the exception.
| @@ -1162,8 +1162,8 @@ | ||
| user_site = site.getusersitepackages() | ||
| if user_site: | ||
| paths.add(os.path.normpath(user_site)) | ||
| except Exception: | ||
| pass | ||
| except Exception as ex: | ||
| warnings.warn(f"Could not import site.getsitepackages() or user site-packages: {ex}") | ||
|
|
||
| # Python prefix paths (covers most installations) | ||
| for prefix in (sys.prefix, sys.base_prefix, sys.exec_prefix): |
jaltmayerpizzorno
left a comment
There was a problem hiding this comment.
I think splitting into commands is a good move!
CLI Overhaul: Verb-based Commands and YAML Configuration
This PR introduces a major refactoring of Scalene's command-line interface to be more intuitive and maintainable.
New Command Structure
Scalene now uses a verb-based CLI with two main commands:
scalene run- ProfilingProfiles a Python program and saves results to JSON (default: scalene-profile.json).
Basic options:
-o/--outfile,--cpu-only,-c/--config,--help-advancedAdvanced options (via
--help-advanced):--profile-all,--profile-only,--profile-exclude,--gpu,--memory,--stacks,--profile-interval,--use-virtual-time,--cpu-percent-threshold,--cpu-sampling-rate,--malloc-threshold,--memory-leak-detector,--on,--offscalene view- Viewing ResultsDisplays an existing profile in browser, terminal, or saves as HTML.
YAML Configuration Files
New support for storing options in a YAML config file:
Usage:
scalene run -c scalene.yaml prog.pyCommand-line arguments override config file settings.
Key Changes
--cpu-only(was--cpu),-oshorthand for--outfile--on/--offmoved to advanced helpFiles Changed
scalene/scalene_parseargs.py- Main CLI implementationscalene/scalene_profiler.py- Updated subprocess cmdline generationscalene/scalene_arguments.py- Updated defaultspyproject.toml- Added pyyaml>=6.0 dependencyREADME.md,index.rst,scalene/scalene-usage.txt- Documentation updatestests/test_coverup_*.py,tests/test_nested_package_relative_import.py- Test updatesMigration Guide