minor maintainance improvements and fixes#296
minor maintainance improvements and fixes#296ParticularlyPythonicBS merged 3 commits intounstablefrom
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 0 minutes and 3 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughThe PR updates documentation and code to clarify the tutorial workflow and improve validation. It modifies instructions for running tutorials and the method of morris extension, treats Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pyproject.toml`:
- Around line 137-140: The current warning filter entry in pyproject.toml
broadly suppresses SyntaxWarning for all mpisppy modules; change that filter to
target only the offending module by replacing the "mpisppy.*" scope with
"mpisppy.utils.sputils" (keeping the rest of the filter string/format intact) so
only SyntaxWarning from mpisppy.utils.sputils is ignored as noted in the
comment.
In `@temoa/cli.py`:
- Around line 691-697: The overwrite-check for mc_settings.csv uses
target_mc_settings = current_dir / 'mc_settings.csv' which doesn't match where
mc_settings.csv is actually created (under target_config.parent based on
config_name); update the check to build target_mc_settings from
target_config.parent (e.g., target_mc_settings = target_config.parent /
'mc_settings.csv') and then use that variable in the existing_files append logic
so overwrite detection aligns with the actual destination used by the code paths
that create mc_settings.csv.
In `@temoa/extensions/method_of_morris/MM_README.md`:
- Around line 37-47: Replace reStructuredText :code:`...` roles in the
MM_README.md with Markdown inline code backticks so they render on GitHub;
specifically update occurrences referencing morris.py, morris_utopia.sql,
morris_utopia.sqlite, morris_utopia.toml and the column names MMAnalysis,
cost_variable, and efficiency to use `morris.py`, `morris_utopia.sql`,
`morris_utopia.sqlite`, `morris_utopia.toml`, `MMAnalysis`, `cost_variable`, and
`efficiency` respectively throughout the document.
In `@temoa/extensions/monte_carlo/example_builds/scenario_analyzer.py`:
- Around line 27-29: The SQL builds a LIKE clause by interpolating scenario_name
into cur.execute (obj_values = cur.execute(...).fetchall()), which is unsafe;
change the call to use a parameterized query and pass the wildcarded pattern as
a parameter (e.g., "SELECT total_system_cost FROM output_objective WHERE
scenario LIKE ?" with parameter f"{scenario_name}-%" or the DB's param style),
so the query is bound instead of using f-string interpolation.
- Around line 34-35: The RuntimeError message that currently reads "Please run
'temoa tutorial model'..." is giving an invalid recovery command; update the
exception text where it's raised (the message referencing output_objective in
scenario_analyzer.py) to instruct running the tutorial model correctly, e.g.,
replace the incorrect string with "Please run 'temoa run tutorial_config.toml'
or run the tutorial model to populate output_objective with results first."
Ensure you update the exact string used in the RuntimeError to reflect the
correct command.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f7206fb8-9973-4f6d-9da0-aefa58d79305
📒 Files selected for processing (6)
docs/source/quick_start.rstpyproject.tomltemoa/cli.pytemoa/extensions/method_of_morris/MM_README.mdtemoa/extensions/monte_carlo/example_builds/scenario_analyzer.pytemoa/tutorial_assets/config_sample.toml
- Update quick start docs to reference 'temoa tutorial' for a complete setup - Add mc_settings.csv to tutorial overwrite detection with correct path resolution - Standardize inline code syntax in Morris extension README for GitHub rendering - Implement parameterized SQL and corrected recovery instructions in Monte Carlo analyzer
Mitigate SyntaxWarning messages triggered by invalid escape sequences in the mpi-sppy dependency. Narrowed the filter scope to target only the offending module (mpisppy.utils.sputils) based on CI logs.
Move the [sqlite] table definition to the end of config_sample.toml. This prevents subsequent root-level keys (like solver_name) from being incorrectly parsed as child keys of the sqlite table.
7509d40 to
7b74cce
Compare
addresses several usability issues related to the Temoa tutorial, extension examples, and overall project maintainability.
Summary by CodeRabbit
Documentation
Bug Fixes
Chores