Uh oh!
There was an error while loading. Please reload this page.
fixing pytest and pre-commit warnings and consistency fixes - #306
Merged
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 10 minutes and 2 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 ignored due to path filters (1)
📒 Files selected for processing (36)
WalkthroughRefactors SQLite connection management to use contextlib.closing(...) and wraps TableWriter usage with with-statements across the codebase. Also applies widespread whitespace/newline normalization, minor formatting changes (type alias, exception quote style), and updates pre-commit hooks to use Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
temoa/extensions/method_of_morris/morris.py (1)
50-57:⚠️ Potential issue | 🔴 CriticalCursor is used after connection close (runtime failure).
cur.execute()at line 56 runs outside thewith contextlib.closing()block that closes at line 53. The connection is already closed, causing a database operational error at runtime. The second query and all cursor operations after line 53 must be moved inside thewithblock.🐛 Proposed fix
with contextlib.closing(sqlite3.connect(db_file)) as con: cur = con.cursor() cur.execute('SELECT * FROM output_objective') output_query = cur.fetchall() + for row in output_query:+ y_of = row[-1]+ cur.execute("SELECT emis_comm, SUM(emission) FROM output_emission WHERE emis_comm='co2'")+ output_query = cur.fetchall()+ for row in output_query:+ y_cumulative_co2 = row[-1]- for row in output_query:- y_of = row[-1]- cur.execute("SELECT emis_comm, SUM(emission) FROM output_emission WHERE emis_comm='co2'")- output_query = cur.fetchall()- for row in output_query:- y_cumulative_co2 = row[-1] morris_objectives = [] morris_objectives.append(y_of) morris_objectives.append(y_cumulative_co2) - con.close() return morris_objectives🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@temoa/extensions/method_of_morris/morris.py` around lines 50 - 57, The code uses cur.execute("SELECT emis_comm, SUM(emission) FROM output_emission WHERE emis_comm='co2'") after the with contextlib.closing(sqlite3.connect(db_file)) as con: block has exited, causing the connection/cur to be closed; move the second query and any cursor operations (the cur variable, the SELECT on output_emission and the subsequent fetchall/use of output_query) inside the same with block where con and cur are valid, ensuring y_of is set and all database access happens before the with block closes (references: contextlib.closing, sqlite3.connect, con, cur, output_query, y_of).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/technology` groups.md:
- Around line 23-24: The sentence fragment beginning "specific names, and the
functionality should remain consistent. For RPS groups..." continues after the
comma with "The migration tool"; change that capitalization to lowercase ("the
migration tool") so the sentence reads as a single continuous sentence; update
the text where the phrase "The migration tool attempts to find a common set of
members..." appears to use "the" instead of "The".
- Line 13: The sentence contains a subject-verb agreement error: replace the
phrase "parameters that accepts" with "parameters that accept" so the plural
noun "parameters" matches the plural verb form; update the sentence fragment
"These group names can now be used in any of the parameters that accepts
technology groups as an index." to use "accept" instead of "accepts".
In `@temoa/cli.py`:
- Around line 587-590: Replace the use of
contextlib.closing(sqlite3.connect(target_database)) with the sqlite3.Connection
context manager so transactions are committed/rolled back properly: change the
"with contextlib.closing(sqlite3.connect(target_database)) as conn:" block that
calls conn.executescript(sql_content) to use "with
sqlite3.connect(target_database) as conn:" (or equivalently use the connection
object's context manager) and apply the same replacement for the fallback SQL
path that also uses contextlib.closing and sqlite3.connect at the other
occurrence; keep using executescript(sql_content) on the conn.
In `@temoa/extensions/modeling_to_generate_alternatives/MGA` Solve Process
Notes.md:
- Around line 30-31: Add a blank line after the markdown headings "####
Alternate (Current) Approach" and "#### Original Intent" so each heading is
followed by an empty line before the paragraph text (to satisfy markdownlint
MD022); locate those headings in the file and insert a single blank line
immediately after each heading line.
---
Outside diff comments:
In `@temoa/extensions/method_of_morris/morris.py`:
- Around line 50-57: The code uses cur.execute("SELECT emis_comm, SUM(emission)
FROM output_emission WHERE emis_comm='co2'") after the with
contextlib.closing(sqlite3.connect(db_file)) as con: block has exited, causing
the connection/cur to be closed; move the second query and any cursor operations
(the cur variable, the SELECT on output_emission and the subsequent fetchall/use
of output_query) inside the same with block where con and cur are valid,
ensuring y_of is set and all database access happens before the with block
closes (references: contextlib.closing, sqlite3.connect, con, cur, output_query,
y_of).
🪄 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: 9ca6652c-ae1d-44f1-99e8-a4d23096cfe6
⛔ Files ignored due to path filters (1)
docs/source/images/TemoaLogo.svgis excluded by!**/*.svg
📒 Files selected for processing (35)
.github/ISSUE_TEMPLATE/bug_report.mddocs/source/References.bibdocs/source/default/layout.htmldocs/source/default/static/google_analytics.jsdocs/source/default/static/my_theme.cssdocs/source/mga.rstdocs/technology groups.mdoutput_files/.gitignoretemoa/_internal/run_actions.pytemoa/_internal/temoa_sequencer.pytemoa/cli.pytemoa/data_processing/README.mdtemoa/extensions/method_of_morris/morris.pytemoa/extensions/method_of_morris/morris_evaluate.pytemoa/extensions/method_of_morris/morris_sequencer.pytemoa/extensions/modeling_to_generate_alternatives/MGA Future Development.mdtemoa/extensions/modeling_to_generate_alternatives/MGA Solve Process Notes.mdtemoa/extensions/modeling_to_generate_alternatives/make_flow_summary_table.sqltemoa/extensions/monte_carlo/mc_sequencer.pytemoa/extensions/monte_carlo/monte carlo design.mdtemoa/extensions/stochastics/scenario_creator.pytemoa/model_checking/unit_checking/screener.pytemoa/types/dict_types.pytemoa/utilities/clear_db_outputs.pytemoa/utilities/master_migration.pytemoa/utilities/run_all_v4_migrations.pytests/conftest.pytests/test_cli.pytests/test_emission_results.pytests/test_full_runs.pytests/test_linked_tech.pytests/test_material_results.pytests/test_unit_propagation.pytests/test_v4_migration.pytests/testing_outputs/README.txt
💤 Files with no reviewable changes (5)
- temoa/data_processing/README.md
- docs/source/References.bib
- output_files/.gitignore
- docs/source/default/static/google_analytics.js
- docs/source/default/layout.html
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
temoa/extensions/method_of_morris/morris.py (1)
49-63: 🧹 Nitpick | 🔵 TrivialRemove redundant close after context-managed connection.
Line 63 manually closes
coneven thoughwith contextlib.closing(...)already owns closure. Keep a single owner for connection lifecycle.♻️ Proposed fix
@@ - con.close() return morris_objectives🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@temoa/extensions/method_of_morris/morris.py` around lines 49 - 63, Remove the redundant manual close: delete the explicit con.close() call and rely on the contextlib.closing(sqlite3.connect(db_file)) as con block to manage connection lifecycle; ensure that any use of con (queries that populate y_of and y_cumulative_co2) happens inside the with block and that morris_objectives (and variables y_of, y_cumulative_co2) are assembled only after those values are retrieved so no code references con after the context manager scope.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@temoa/extensions/method_of_morris/morris.py`:
- Around line 49-63: Remove the redundant manual close: delete the explicit
con.close() call and rely on the contextlib.closing(sqlite3.connect(db_file)) as
con block to manage connection lifecycle; ensure that any use of con (queries
that populate y_of and y_cumulative_co2) happens inside the with block and that
morris_objectives (and variables y_of, y_cumulative_co2) are assembled only
after those values are retrieved so no code references con after the context
manager scope.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ca011943-ec7d-475d-8a27-b96d71a38f0d
📒 Files selected for processing (5)
.pre-commit-config.yamldocs/technology groups.mdtemoa/cli.pytemoa/extensions/method_of_morris/morris.pytemoa/extensions/modeling_to_generate_alternatives/MGA Solve Process Notes.md
- Wrapped short-lived connections in context handlers - Bound TableWriter lifespan correctly for reliable closures - Replaced explicit contextlib with core SQLite context managers in generation logic
…across remaining resources
ParticularlyPythonicBSforce-pushed
the
fix/pytest_warning
branch
from
April 10, 2026 18:19
9c31b98 to
b06a653CompareUh oh!
There was an error while loading. Please reload this page.
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
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary by CodeRabbit
Chores
Refactor
Tests