removing and mitigating all remaining references to data_files#295
Merged
ParticularlyPythonicBS merged 1 commit intounstablefrom Apr 8, 2026
Merged
Conversation
ParticularlyPythonicBS
added
the
Maintenance
WalkthroughReplaced legacy packaged-resource references and data_files paths with direct filesystem paths; added existence checks and explicit errors; updated docs and tutorial assets to reflect new resource locations and added a new tutorial CSV to copied resources. Changes
Sequence Diagram(s)(Skipped — changes do not introduce a new multi-component sequential flow requiring visualization.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@temoa/extensions/method_of_morris/MM_README.md`:
- Around line 37-42: The markdown list item formatting is broken because the
inline shell command `sqlite3 morris_utopia.sqlite < morris_utopia.sql` is not
nested under step 1; re-indent or fence that command under step 1 so it is a
child of the first ordered item (e.g., indent the command by four spaces or wrap
it in a fenced code block) and ensure the rest of the numbered steps
(observations about MMAnalysis and morris config) remain at the same list level.
In `@temoa/extensions/method_of_morris/morris.py`:
- Around line 70-78: The example asset lookup currently uses db_file and
config_path as Path('morris_utopia.sqlite') and Path('morris_utopia.toml'),
which relies on the process CWD; change them to resolve relative to the module
root (the same base used for param_file which uses Path(__file__).parent or an
existing morris_root) or allow callers to pass explicit paths. Specifically,
replace the plain Path(...) construction for db_file and config_path with
Path(__file__).parent.joinpath('morris_utopia.sqlite') and
Path(__file__).parent.joinpath('morris_utopia.toml') (or use the existing
morris_root variable if present), and update the existence check and
FileNotFoundError message accordingly so the files are found relative to the
package rather than the caller's working directory.
In `@temoa/extensions/monte_carlo/example_builds/scenario_analyzer.py`:
- Around line 11-14: The README/hint and runtime behavior are misleading: the
comment and variable db_resource use 'utopia.sqlite' but the tutorial command
creates 'tutorial_database.sqlite', and opening a non-existent DB creates an
empty file causing later "no such table" errors; update the setup hint to show
the exact command that creates utopia.sqlite, and add an explicit existence
check for db_resource before calling Connection (e.g., check
os.path.exists(db_resource) or pathlib.Path(db_resource).exists() and raise a
clear error if missing) so the code using Connection(db_resource) fails fast
with a helpful message; refer to the db_resource variable and the
Connection(...) context to locate where to change the comment and insert the
existence check.
In `@temoa/tutorial_assets/config_sample.toml`:
- Around line 188-190: The sample config's [monte_carlo] run_settings key points
to 'mc_settings.csv', which the tutorial CLI handler (the tutorial command in
temoa/cli.py) does not create or copy, causing a missing-file failure; fix by
either updating the sample to reference a package-resolved path to the bundled
mc_settings.csv (so run_settings points to the resource shipped with the
package) or modify the tutorial command to copy the bundled mc_settings.csv into
the tutorial output directory alongside the generated TOML/SQLite (ensure you
update the run_settings value to the copied filename or the correct relative
path).
In `@temoa/utilities/db_migration_to_v3.py`:
- Around line 26-29: The default value for the CLI argument with dest='schema'
currently uses a cwd-dependent string ('temoa/db_schema/temoa_schema_v3.sql')
which can fail when the script is run from another working directory; update the
argument default and the schema file loader in db_migration_to_v3.py to resolve
the schema path relative to the temoa package (use the same module-relative
resource-loading approach as temoa/cli.py, e.g., importlib.resources or the
project's helper used there) so that the code that opens the schema file (the
code that reads the 'schema' argument and calls open/read) always finds the
schema regardless of CWD; ensure the parser default points to a resolved
absolute/module resource path or make the argument default None and fallback to
the module-relative path when loading.
🪄 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: a8c08822-fd0b-47a4-bae8-3d038aaf1cea
📒 Files selected for processing (9)
.gitignoredocs/source/computational_implementation.rstdocs/source/quick_start.rsttemoa/extensions/method_of_morris/MM_README.mdtemoa/extensions/method_of_morris/Method_of_Morris_README.txttemoa/extensions/method_of_morris/morris.pytemoa/extensions/monte_carlo/example_builds/scenario_analyzer.pytemoa/tutorial_assets/config_sample.tomltemoa/utilities/db_migration_to_v3.py
💤 Files with no reviewable changes (1)
- .gitignore
ParticularlyPythonicBS
force-pushed
the
maintainance/state_data_file_references
branch
from
8a9c7ac to
b8d7e89
Compare
ParticularlyPythonicBS
force-pushed
the
maintainance/state_data_file_references
branch
from
b8d7e89 to
3fc8abb
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/source/quick_start.rst`:
- Around line 101-102: Update the sentence that references the packaged
config_sample.toml to point readers to the temoa tutorial rather than implying
the packaged file in temoa/tutorial_assets is directly runnable; mention that
config_sample.toml alone is insufficient because the tutorial flow also
generates the SQLite database and copies mc_settings.csv alongside it, and
instruct users to follow the temoa tutorial to obtain a fully runnable setup
(reference config_sample.toml and temoa tutorial by name to locate the correct
docs).
In `@temoa/cli.py`:
- Around line 590-594: The helper writes a third file named "mc_settings.csv"
into the target directory but the tutorial() preflight only checks config and
DB; to avoid silently overwriting user-edited files, add the target path for
mc_settings.csv into existing_files before calling the helper. Concretely,
compute the path the helper uses (target_config.parent / 'mc_settings.csv' or
the same expression used in the helper) and append it to the existing_files
collection used by tutorial(), so the overwrite checks include
"mc_settings.csv".
In `@temoa/extensions/method_of_morris/MM_README.md`:
- Around line 37-44: The README omits that the example assets must live next to
the extension code; update the instructions to tell users to run the sqlite
conversion and place both morris_utopia.sqlite and morris_utopia.toml in the
temoa/extensions/method_of_morris directory because morris.py resolves those
files via Path(__file__).parent; include steps to obtain/create
morris_utopia.sql (or download) and to ensure morris_utopia.toml is present in
the same directory before running the example.
In `@temoa/extensions/monte_carlo/example_builds/scenario_analyzer.py`:
- Around line 12-22: The setup check only ensures tutorial_database.sqlite
exists (db_resource) but doesn't verify the DB has analyzable results; open the
DB via Connection(db_resource), run the same query that selects rows from
output_objective (the query that filters for "Purple Onion-*" or equivalent) and
if the result count is zero raise a clear error asking the user to run the
tutorial model (e.g., "Please run 'temoa tutorial model' or the tutorial model
to populate output_objective with 'Purple Onion-*' rows"); also update the
initial help text that references db_resource/Path to instruct running the
tutorial model first. Use identifiers Path, db_resource, and Connection when
locating code to change.
🪄 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: 533690a4-6c07-43c3-b3e4-4b201bc85921
⛔ Files ignored due to path filters (1)
temoa/tutorial_assets/mc_settings.csvis excluded by!**/*.csv
📒 Files selected for processing (10)
.gitignoredocs/source/computational_implementation.rstdocs/source/quick_start.rsttemoa/cli.pytemoa/extensions/method_of_morris/MM_README.mdtemoa/extensions/method_of_morris/Method_of_Morris_README.txttemoa/extensions/method_of_morris/morris.pytemoa/extensions/monte_carlo/example_builds/scenario_analyzer.pytemoa/tutorial_assets/config_sample.tomltemoa/utilities/db_migration_to_v3.py
💤 Files with no reviewable changes (1)
- .gitignore
Comment on lines
+101
to
+102
| example config file called :code:`config_sample.toml` is available as a package resource | ||
| in :code:`temoa/tutorial_assets`. Running the model with a config file allows |
There was a problem hiding this comment.
Point readers to temoa tutorial, not the internal resource path.
config_sample.toml under temoa/tutorial_assets is not runnable by itself; the tutorial flow also generates the SQLite database and copies mc_settings.csv alongside it. As written, this suggests users can use the packaged file directly.
📝 Suggested doc tweak
-To run the model, a configuration (``config``) file is needed. An
-example config file called :code:`config_sample.toml` is available as a package resource
-in :code:`temoa/tutorial_assets`. Running the model with a config file allows
+To run the model, a configuration (``config``) file is needed. The supported
+way to create the sample config and its companion tutorial assets is:
+
+.. parsed-literal::
+ $ temoa tutorial my_first_model
+
+This copies :code:`config_sample.toml`, generates the SQLite tutorial database,
+and copies :code:`mc_settings.csv` into the working directory. Running the model with a config file allows📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| example config file called :code:`config_sample.toml` is available as a package resource | |
| in :code:`temoa/tutorial_assets`. Running the model with a config file allows | |
| To run the model, a configuration (``config``) file is needed. The supported | |
| way to create the sample config and its companion tutorial assets is: | |
| .. parsed-literal:: | |
| $ temoa tutorial my_first_model | |
| This copies :code:`config_sample.toml`, generates the SQLite tutorial database, | |
| and copies :code:`mc_settings.csv` into the working directory. Running the model with a config file allows |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/source/quick_start.rst` around lines 101 - 102, Update the sentence that
references the packaged config_sample.toml to point readers to the temoa
tutorial rather than implying the packaged file in temoa/tutorial_assets is
directly runnable; mention that config_sample.toml alone is insufficient because
the tutorial flow also generates the SQLite database and copies mc_settings.csv
alongside it, and instruct users to follow the temoa tutorial to obtain a fully
runnable setup (reference config_sample.toml and temoa tutorial by name to
locate the correct docs).
Comment on lines
+590
to
+594
| # Copy Monte Carlo settings | ||
| with mc_settings_resource.open('rb') as source: | ||
| target_mc = target_config.parent / 'mc_settings.csv' | ||
| with open(target_mc, 'wb') as target: | ||
| shutil.copyfileobj(source, target) |
There was a problem hiding this comment.
Include mc_settings.csv in the tutorial overwrite checks.
This helper now writes a third file into the target directory, but tutorial() still only checks the config and database for pre-existing conflicts. A user-edited mc_settings.csv will be silently clobbered on reruns. Add it to existing_files before calling this helper.
🛡️ Suggested fix
def tutorial(
@@
current_dir = Path.cwd()
target_config = current_dir / f'{config_name}.toml'
target_database = current_dir / f'{database_name}.sqlite'
+ target_mc = current_dir / 'mc_settings.csv'
# Check for existing files and handle conflicts
existing_files = []
if target_config.exists():
existing_files.append(str(target_config))
if target_database.exists():
existing_files.append(str(target_database))
+ if target_mc.exists():
+ existing_files.append(str(target_mc))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@temoa/cli.py` around lines 590 - 594, The helper writes a third file named
"mc_settings.csv" into the target directory but the tutorial() preflight only
checks config and DB; to avoid silently overwriting user-edited files, add the
target path for mc_settings.csv into existing_files before calling the helper.
Concretely, compute the path the helper uses (target_config.parent /
'mc_settings.csv' or the same expression used in the helper) and append it to
the existing_files collection used by tutorial(), so the overwrite checks
include "mc_settings.csv".
Comment on lines
+37
to
+44
| 1. Convert the `.sql` source file to a database. If you have the `morris_utopia.sql` file: | ||
|
|
||
| `data_files/example_dbs % sqlite3 morris_utopia.sqlite < morris_utopia.sql` | ||
| ```bash | ||
| sqlite3 morris_utopia.sqlite < morris_utopia.sql | ||
| ``` | ||
|
|
||
| 2. Observe the markings (3 groups) in the `MMAnalysis` columns in `cost_variable` and `efficiency`. | ||
| 3. Observe the `morris` configuration comments in the corresponding `morris_utopia.toml` file in `my_configs`. | ||
| 3. Observe the `morris` configuration comments in the relevant config file (e.g., `morris_utopia.toml`). |
There was a problem hiding this comment.
Specify where the Morris example assets must live.
morris.py now resolves both morris_utopia.sqlite and morris_utopia.toml from Path(__file__).parent, so generating the database in an arbitrary shell directory still leaves the example broken. This section also assumes the reader already has those files. Please document the required destination directory and how to obtain/create both assets here.
📝 Suggested doc update
-1. Convert the `.sql` source file to a database. If you have the `morris_utopia.sql` file:
+1. Place the Morris example assets next to `morris.py` in `temoa/extensions/method_of_morris/`.
+ If you have the `morris_utopia.sql` source, generate the database in that directory:
```bash
+ cd temoa/extensions/method_of_morris
sqlite3 morris_utopia.sqlite < morris_utopia.sql
```
+ Ensure `morris_utopia.toml` is present in the same directory before running the example.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@temoa/extensions/method_of_morris/MM_README.md` around lines 37 - 44, The
README omits that the example assets must live next to the extension code;
update the instructions to tell users to run the sqlite conversion and place
both morris_utopia.sqlite and morris_utopia.toml in the
temoa/extensions/method_of_morris directory because morris.py resolves those
files via Path(__file__).parent; include steps to obtain/create
morris_utopia.sql (or download) and to ensure morris_utopia.toml is present in
the same directory before running the example.
Comment on lines
+12
to
+22
| # To run this example, ensure tutorial_database.sqlite is in your current directory. | ||
| # You can generate it using: temoa tutorial | ||
| db_resource = 'tutorial_database.sqlite' | ||
|
|
||
| if not Path(db_resource).exists(): | ||
| raise FileNotFoundError( | ||
| f"Database file '{db_resource}' not found. " | ||
| "Please run 'temoa tutorial' to create the required files." | ||
| ) | ||
|
|
||
| with Connection(db_resource) as conn: |
There was a problem hiding this comment.
temoa tutorial alone does not produce analyzable results.
This only guarantees that tutorial_database.sqlite exists. It does not populate output_objective with Purple Onion-* rows, so the query can return nothing and the later histogram call trips on bins=0. Update the setup hint to require running the tutorial model first, and fail fast when no matching rows are found.
📝 Suggested fix
-# To run this example, ensure tutorial_database.sqlite is in your current directory.
-# You can generate it using: temoa tutorial
+# To run this example, first create the tutorial assets and then run the
+# tutorial model so `output_objective` contains `Purple Onion-*` rows:
+# temoa tutorial
+# temoa run tutorial_config.toml
...
obj_values = cur.execute(
f"SELECT total_system_cost FROM output_objective WHERE scenario LIKE '{scenario_name}-%'"
).fetchall()
+ if not obj_values:
+ raise RuntimeError(
+ f"No results found for '{scenario_name}'. Run the tutorial model before analyzing it."
+ )🧰 Tools
🪛 Ruff (0.15.9)
[warning] 17-20: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@temoa/extensions/monte_carlo/example_builds/scenario_analyzer.py` around
lines 12 - 22, The setup check only ensures tutorial_database.sqlite exists
(db_resource) but doesn't verify the DB has analyzable results; open the DB via
Connection(db_resource), run the same query that selects rows from
output_objective (the query that filters for "Purple Onion-*" or equivalent) and
if the result count is zero raise a clear error asking the user to run the
tutorial model (e.g., "Please run 'temoa tutorial model' or the tutorial model
to populate output_objective with 'Purple Onion-*' rows"); also update the
initial help text that references db_resource/Path to instruct running the
tutorial model first. Use identifiers Path, db_resource, and Connection when
locating code to change.
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 free
to 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