Conversation
…ict instance for the deprecated raster class
…e raster class from docs
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRemoved the Raster module and Raster re-export, deleted Scenario.datasheet_rasters, removed rasterio from dependencies, deleted related autosummary docs, updated example/tests to open rasters directly (datasheet-driven approach). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
examples/spatial_demo.py (2)
167-191:⚠️ Potential issue | 🟠 MajorDemo code below TODO is broken and will raise
AttributeErrorat runtime.
datasheet_rastersnow returns dictionaries with keys"rpath","iteration", and"timestep"instead ofRasterobjects. The following calls will fail:
spatialRaster.values()→ dicts have no.values()method that returns cell dataspatialRaster.source→ dicts have no.sourceattribute (usespatialRaster["rpath"]instead)spatialRasters[15].values()→ same issueConsider either completing the update now or commenting out the broken code to prevent confusion.
💡 Example of updated usage with new dict-based API
# Get raster path from dict raster_path = spatialRaster["rpath"] # Use rasterio directly (if reinstated as optional dependency) with rasterio.open(raster_path) as raster: cell_values = raster.read() pyplot.imshow(cell_values[0])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/spatial_demo.py` around lines 167 - 191, The demo code after the TODO uses Raster instance methods/attrs but datasheet_rasters now returns dicts; update the snippet to use the new dict-based API or remove/comment it: replace uses of spatialRaster.values(), spatialRaster.source and spatialRasters[15].values() with extracting the raster path via spatialRaster["rpath"] (and spatialRasters[i]["rpath"]) and then open with rasterio (e.g., with rasterio.open(spatialRaster["rpath"]) to read and pyplot.imshow the band), or simply comment out the broken block until Raster objects are reinstated; reference datasheet_rasters, spatialRaster, spatialRasters, values(), and source when making the changes.
10-10:⚠️ Potential issue | 🔴 Critical
rasterioimport will fail at runtime since it's no longer a dependency.The
rasteriopackage was removed fromsetup.pyandrequirements.txt, so this import will raiseModuleNotFoundErrorwhen running the demo.🐛 Proposed fix: Remove the import or update the demo
-import rasterioNote: The TODO at line 167 acknowledges this file needs updating, but leaving a broken import in the codebase means the demo cannot be executed at all until it's fixed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/spatial_demo.py` at line 10, Top-level import "import rasterio" will raise ModuleNotFoundError because rasterio is no longer a dependency; remove this import and either (A) remove or replace any code that uses rasterio in this demo, or (B) make rasterio an optional dependency by moving the import into the specific function(s) that need it and surrounding it with a try/except that raises a clear runtime message instructing users to install rasterio if they attempt to run that part. Update any functions referencing rasterio accordingly so the module can be executed without rasterio installed (or fails with a clear message when rasterio functionality is invoked).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/test_pysyncrosim.py`:
- Around line 15-16: Replace the hardcoded user-specific string assigned to
git_repo_path in tests/test_pysyncrosim.py with a portable resolution: read a
fallback from an environment variable (e.g., GIT_REPO_PATH) and if not set,
derive a path relative to the test file using the test module's location (e.g.,
Path(__file__).resolve().parent) so the test works on other machines and CI;
update any references/tests that use git_repo_path to accept the new resolution
method.
---
Outside diff comments:
In `@examples/spatial_demo.py`:
- Around line 167-191: The demo code after the TODO uses Raster instance
methods/attrs but datasheet_rasters now returns dicts; update the snippet to use
the new dict-based API or remove/comment it: replace uses of
spatialRaster.values(), spatialRaster.source and spatialRasters[15].values()
with extracting the raster path via spatialRaster["rpath"] (and
spatialRasters[i]["rpath"]) and then open with rasterio (e.g., with
rasterio.open(spatialRaster["rpath"]) to read and pyplot.imshow the band), or
simply comment out the broken block until Raster objects are reinstated;
reference datasheet_rasters, spatialRaster, spatialRasters, values(), and source
when making the changes.
- Line 10: Top-level import "import rasterio" will raise ModuleNotFoundError
because rasterio is no longer a dependency; remove this import and either (A)
remove or replace any code that uses rasterio in this demo, or (B) make rasterio
an optional dependency by moving the import into the specific function(s) that
need it and surrounding it with a try/except that raises a clear runtime message
instructing users to install rasterio if they attempt to run that part. Update
any functions referencing rasterio accordingly so the module can be executed
without rasterio installed (or fails with a clear message when rasterio
functionality is invoked).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0c294c27-87fe-44a2-9ede-2065c497548f
📒 Files selected for processing (18)
docs/_autosummary/pysyncrosim.raster.Raster.__init__.rstdocs/_autosummary/pysyncrosim.raster.Raster.crs.rstdocs/_autosummary/pysyncrosim.raster.Raster.dimensions.rstdocs/_autosummary/pysyncrosim.raster.Raster.extent.rstdocs/_autosummary/pysyncrosim.raster.Raster.name.rstdocs/_autosummary/pysyncrosim.raster.Raster.resolution.rstdocs/_autosummary/pysyncrosim.raster.Raster.rstdocs/_autosummary/pysyncrosim.raster.Raster.source.rstdocs/_autosummary/pysyncrosim.raster.Raster.values.rstdocs/module_summary.rstexamples/spatial_demo.pypysyncrosim/__init__.pypysyncrosim/raster.pypysyncrosim/scenario.pyrequirements.txtsetup.pytests/spatial-example.ssimbaktests/test_pysyncrosim.py
💤 Files with no reviewable changes (13)
- requirements.txt
- docs/_autosummary/pysyncrosim.raster.Raster.crs.rst
- docs/module_summary.rst
- docs/_autosummary/pysyncrosim.raster.Raster.name.rst
- docs/_autosummary/pysyncrosim.raster.Raster.resolution.rst
- docs/_autosummary/pysyncrosim.raster.Raster.init.rst
- docs/_autosummary/pysyncrosim.raster.Raster.rst
- pysyncrosim/init.py
- docs/_autosummary/pysyncrosim.raster.Raster.values.rst
- docs/_autosummary/pysyncrosim.raster.Raster.source.rst
- docs/_autosummary/pysyncrosim.raster.Raster.extent.rst
- docs/_autosummary/pysyncrosim.raster.Raster.dimensions.rst
- pysyncrosim/raster.py
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/spatial_demo.py`:
- Around line 157-158: Fix the typo in the inline comment that starts with "#
Get a raster (returns a dicionary with keys for the path "rpath"," by changing
"dicionary" to "dictionary"; locate the comment that mentions the keys "rpath",
"iteration", and "timestep" and update the spelling only (no logic/code
changes).
- Around line 168-171: The standalone expression `cell_values` is flagged as a
useless expression; update the raster read block using
`rasterio.open(spatialRaster["rpath"])` so that `cell_values` is either used or
explicitly displayed—either remove the bare expression, replace it with an
explicit `print(cell_values)` if you intend to show the values, or add a clear
comment (e.g., "# Intentional: show cell_values in interactive session") to
silence the linter while keeping the existing `cell_values` variable.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/spatial_demo.py`:
- Around line 177-185: Implement a public datasheet_rasters method on the
Scenario class that validates inputs using __validate_datasheet_raster_inputs,
calls __retrieve_raster_column to fetch the raw raster column values, uses
__list_datasheet_rasters to resolve or copy files as needed, and returns a list
of dictionaries where each dict contains at least the "rpath" key pointing to
the local raster path; ensure the new Scenario.datasheet_rasters wraps the
existing helpers, accepts the same parameters used in the example (datasheet,
column), and raises or forwards validation errors when helpers indicate invalid
inputs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 74c23a58-1b63-4091-9bf0-d95273cae959
📒 Files selected for processing (2)
examples/spatial_demo.pypysyncrosim/scenario.py
💤 Files with no reviewable changes (1)
- pysyncrosim/scenario.py
examples/spatial_demo.py
Outdated
| # Get multiple rasters in a list (returns a list of dicts) | ||
| spatialRasters = myResultsScenario.datasheet_rasters( | ||
| datasheet = "helloworldSpatial_IntermediateDatasheet", | ||
| column = "OutputRasterFile") | ||
|
|
||
| spatialRasters[15] | ||
|
|
||
| pyplot.imshow(spatialRasters[15].values(), cmap = "pink") | ||
| with rasterio.open(spatialRasters[15]["rpath"]) as raster: | ||
| pyplot.imshow(raster.read(1), cmap = "pink") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the public datasheet_rasters method exists and check its return type construction
# Search for the public datasheet_rasters method definition
echo "=== Searching for datasheet_rasters method definition ==="
ast-grep --pattern $'def datasheet_rasters($$$):
$$$'
# Check how the method constructs its return value
echo ""
echo "=== Checking return statements in scenario.py ==="
rg -n -A 10 'def datasheet_rasters' --type pyRepository: syncrosim/pysyncrosim
Length of output: 173
🏁 Script executed:
# Search for datasheet_rasters in all files (not just method definitions)
echo "=== Searching for 'datasheet_rasters' in entire codebase ==="
rg -i "datasheet_rasters" --type py -n
# Check the example file to understand context
echo ""
echo "=== Checking examples/spatial_demo.py around lines 177-185 ==="
cat -n examples/spatial_demo.py | sed -n '170,190p'Repository: syncrosim/pysyncrosim
Length of output: 3049
🏁 Script executed:
# Search for the method in scenario.py more broadly (may be property or use decorators)
echo "=== Searching in scenario.py for datasheet_rasters pattern ==="
rg -n "datasheet_rasters" pysyncrosim/scenario.py -B 3 -A 15
# Look for the Scenario class definition and its methods
echo ""
echo "=== Checking Scenario class structure ==="
rg -n "class Scenario" pysyncrosim/scenario.py -A 5
# Check if it's dynamically added or inherited
echo ""
echo "=== Checking imports and base classes in scenario.py ==="
head -50 pysyncrosim/scenario.pyRepository: syncrosim/pysyncrosim
Length of output: 2709
🏁 Script executed:
# Get the size of scenario.py
wc -l pysyncrosim/scenario.py
# Search for all method definitions in scenario.py
echo "=== All def statements in scenario.py ==="
rg -n "^\s*def " pysyncrosim/scenario.py
# Search for `@property` decorator before datasheet
echo ""
echo "=== Searching for property decorators in scenario.py ==="
rg -n "@property" pysyncrosim/scenario.py -A 1Repository: syncrosim/pysyncrosim
Length of output: 2972
🏁 Script executed:
# Search for datasheet_rasters in all Python files in pysyncrosim package
echo "=== Searching entire pysyncrosim package for datasheet_rasters ==="
rg "def datasheet_rasters" pysyncrosim/ -n
# Also check if it's dynamically added or inherited
echo ""
echo "=== Checking __init__.py files ==="
find pysyncrosim -name "__init__.py" -exec grep -l "datasheet_rasters" {} \;
# List all Python files in pysyncrosim
echo ""
echo "=== Python files in pysyncrosim ==="
find pysyncrosim -name "*.py" -type f | head -20Repository: syncrosim/pysyncrosim
Length of output: 416
🏁 Script executed:
# Check the test file to understand context of datasheet_rasters calls
echo "=== Test file context around datasheet_rasters ==="
rg -n "myResultsScenario" tests/test_pysyncrosim.py | head -20
# Get more context from test - what is myResultsScenario
echo ""
echo "=== Getting test context ==="
sed -n '925,960p' tests/test_pysyncrosim.py
# Check if method might be defined elsewhere or if it's genuinely missing
echo ""
echo "=== Complete grep for datasheet_rasters pattern ==="
grep -r "datasheet_rasters" pysyncrosim/ || echo "NOT FOUND IN PYSYNCROSIM"Repository: syncrosim/pysyncrosim
Length of output: 3559
🏁 Script executed:
# Check if tests are skipped or marked as expected failures
echo "=== Checking if test_datasheet_rasters tests are skipped ==="
sed -n '930,1050p' tests/test_pysyncrosim.py | grep -E "@pytest|@skip|xfail|TODO"
# Check for any comments about implementation status
echo ""
echo "=== Searching for TODO/FIXME related to datasheet_rasters ==="
rg -i "todo|fixme" pysyncrosim/scenario.py | head -10
# Check git log to see if method was recently removed or planned
echo ""
echo "=== Checking for any implementation placeholders ==="
sed -n '800,850p' pysyncrosim/scenario.pyRepository: syncrosim/pysyncrosim
Length of output: 2678
🏁 Script executed:
# Let me check if there's a method stub or if it might be defined with a different pattern
echo "=== Searching for all public methods (not starting with __) in Scenario ==="
rg -n "^\s+def [a-z]" pysyncrosim/scenario.py | tail -20
# Check end of scenario.py to see if there's anything at the end
echo ""
echo "=== Last 50 lines of scenario.py ==="
tail -50 pysyncrosim/scenario.py
# Double-check: search for the exact method name in all files
echo ""
echo "=== Final search for datasheet_rasters definition ==="
find . -name "*.py" -type f -exec grep -l "def datasheet_rasters" {} \;Repository: syncrosim/pysyncrosim
Length of output: 2691
The datasheet_rasters method is not implemented in the Scenario class.
This example code cannot work as written. The method datasheet_rasters is called on line 178 but does not exist in pysyncrosim/scenario.py. While helper methods like __validate_datasheet_raster_inputs, __retrieve_raster_column, and __list_datasheet_rasters are present, the public datasheet_rasters method that wraps these and returns a list of dictionaries with "rpath" keys is missing entirely. The code will fail at runtime with an AttributeError. This method needs to be implemented to make the example functional.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/spatial_demo.py` around lines 177 - 185, Implement a public
datasheet_rasters method on the Scenario class that validates inputs using
__validate_datasheet_raster_inputs, calls __retrieve_raster_column to fetch the
raw raster column values, uses __list_datasheet_rasters to resolve or copy files
as needed, and returns a list of dictionaries where each dict contains at least
the "rpath" key pointing to the local raster path; ensure the new
Scenario.datasheet_rasters wraps the existing helpers, accepts the same
parameters used in the example (datasheet, column), and raises or forwards
validation errors when helpers indicate invalid inputs.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Checking current state of examples/spatial_demo.py for datasheet_rasters ==="
grep -n "datasheet_rasters" examples/spatial_demo.py || echo "NOT FOUND in spatial_demo.py"
echo ""
echo "=== Current View spatial results section ==="
grep -n "datasheet_rasters\|spatialRaster\|rasterio\|rpath" examples/spatial_demo.py | head -30
Length of output: 786
🏁 Script executed:
#!/bin/bash
echo "=== Checking for datasheet_rasters definition in scenario.py ==="
grep -n "def datasheet_rasters\|datasheet_rasters" pysyncrosim/scenario.py | head -20
echo ""
echo "=== Checking all Python files in pysyncrosim for datasheet_rasters ==="
grep -rn "def datasheet_rasters" pysyncrosim/ || echo "NOT FOUND"
Length of output: 212
@vickimzhang, it looks like the issue is still present. The datasheet_rasters calls at lines 159 and 178 of examples/spatial_demo.py remain, but the public datasheet_rasters method is still not defined anywhere in pysyncrosim/scenario.py — only the private helper __list_datasheet_rasters exists (line 836). Running this example would still raise an AttributeError.
To clarify my original comment: the concern wasn't about the Raster class usage (which you've correctly removed), but about the missing public datasheet_rasters method that the example relies on.
You have two options to resolve this:
- Implement the public
datasheet_rastersmethod inpysyncrosim/scenario.pythat wraps the existing private helpers and returns a list of dicts with"rpath"keys (as the example expects). - Remove or rewrite the spatial results section in
examples/spatial_demo.pyto not usedatasheet_rastersif the method is being fully deprecated alongside the raster class.
Would you like me to help implement either approach?
There was a problem hiding this comment.
should be removed now
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Checking current state of examples/spatial_demo.py for datasheet_rasters ==="
grep -n "datasheet_rasters\|spatialRaster\|rpath" examples/spatial_demo.py || echo "NOT FOUND in spatial_demo.py"
echo ""
echo "=== Checking for public datasheet_rasters definition in scenario.py ==="
grep -n "def datasheet_rasters" pysyncrosim/scenario.py || echo "NOT FOUND in scenario.py"
echo ""
echo "=== Checking full View spatial results section ==="
grep -n -A 2 -B 2 "spatial" examples/spatial_demo.py | tail -30
Length of output: 1638
@vickimzhang, the datasheet_rasters calls are still present in examples/spatial_demo.py (lines 159 and 178), and the public datasheet_rasters method is still not defined in pysyncrosim/scenario.py. The issue persists — running this example would still raise an AttributeError at runtime.
The two calls that remain:
- Line 159:
myResultsScenario.datasheet_rasters(datasheet="IntermediateDatasheet", column="OutputRasterFile", iteration=3, timestep=4) - Line 178:
myResultsScenario.datasheet_rasters(datasheet="helloworldSpatial_IntermediateDatasheet", column="OutputRasterFile")
Could you either:
- Remove the entire "View spatial results" section (lines 155–186) from
examples/spatial_demo.pyif this functionality is being deprecated, or - Implement the public
datasheet_rastersmethod inpysyncrosim/scenario.pyif it's still needed?
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)
tests/test_pysyncrosim.py (1)
7-7:⚠️ Potential issue | 🔴 CriticalRemove unused
rasterioimport that will cause ImportError.The PR removes
rasteriofrom requirements and setup, but this import remains. Since all raster-related tests were deleted, this import is now unused and will cause tests to fail withModuleNotFoundErroronce the dependency is removed.🐛 Proposed fix
-import rasterio🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_pysyncrosim.py` at line 7, Remove the unused import of rasterio from tests/test_pysyncrosim.py: delete the top-level "import rasterio" statement so the test module no longer depends on the removed dependency; verify there are no other references to rasterio in the file (e.g., functions or fixtures) and run tests to ensure no missing imports remain.
🤖 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 `@tests/test_pysyncrosim.py`:
- Line 7: Remove the unused import of rasterio from tests/test_pysyncrosim.py:
delete the top-level "import rasterio" statement so the test module no longer
depends on the removed dependency; verify there are no other references to
rasterio in the file (e.g., functions or fixtures) and run tests to ensure no
missing imports remain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9566f28b-d5c8-4826-a398-8503469c2ed8
📒 Files selected for processing (3)
LICENSEdocs/_autosummary/pysyncrosim.scenario.Scenario.rsttests/test_pysyncrosim.py
💤 Files with no reviewable changes (1)
- docs/_autosummary/pysyncrosim.scenario.Scenario.rst
✅ Files skipped from review due to trivial changes (1)
- LICENSE
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/conf.py (1)
138-139:⚠️ Potential issue | 🟡 MinorRemove stale
rasteriomock import from Sphinx config.
rasteriois still listed inautodoc_mock_imports(Line 138), which conflicts with the PR goal of removing raster/rasterio references.Suggested diff
-autodoc_mock_imports = ['numpy', 'pandas', 'os', 're', 'io', 'tempfile', - 'shutil', 'rasterio', 'subprocess'] +autodoc_mock_imports = ['numpy', 'pandas', 'os', 're', 'io', 'tempfile', + 'shutil', 'subprocess']🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/conf.py` around lines 138 - 139, Remove the stale "rasterio" entry from the autodoc_mock_imports list in the Sphinx config (the autodoc_mock_imports symbol) so the project no longer mocks or references rasterio; update the autodoc_mock_imports list to omit "rasterio" and keep the remaining imports unchanged.examples/spatial_demo.py (1)
10-11:⚠️ Potential issue | 🔴 CriticalAdd rasterio as an optional dependency or document the requirement.
The example script imports and uses rasterio at multiple points (lines 169–176, 183) to open and display raster data. However, rasterio has been removed from
requirements.txtandsetup.pyand is not included inextras_require. Users running this example will encounter anImportError.Resolve by either:
- Adding rasterio to
extras_require(e.g.,"spatial": ["rasterio"]) to make it an optional dependency- Adding a note in the example file or documentation that rasterio must be installed separately
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/spatial_demo.py` around lines 10 - 11, The example imports the rasterio module (symbol: rasterio) but rasterio was removed from package dependencies, so either add it as an optional dependency named "spatial" (e.g., in setup.py/pyproject extras_require add "spatial": ["rasterio"]) so users can pip install it via extras, or update the example (spatial_demo.py) to document the requirement by adding a top-of-file comment and/or a try/except around "import rasterio" that raises a clear message instructing users to pip install rasterio; implement one of these fixes and keep the import symbol rasterio unchanged in the demo.
🧹 Nitpick comments (1)
examples/spatial_demo.py (1)
182-184: Consider clarifying or computing the index instead of hardcoding.The index
15is a magic number that assumes specific run parameters (5 iterations × 10 timesteps = 50 rasters). If someone modifiesRunControlat lines 71-74, this could raise anIndexError.Consider adding a brief comment explaining the choice, or computing the index dynamically:
# Access a specific raster (e.g., middle of the list) idx = len(spatialRasters) // 2 with rasterio.open(spatialRasters[idx]) as raster:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/spatial_demo.py` around lines 182 - 184, Replace the hardcoded index 15 when accessing spatialRasters to avoid IndexError if RunControl changes: compute the desired index from the list (e.g., middle or a named selection) using len(spatialRasters) and store it in a variable (referencing spatialRasters and the rasterio.open usage) or at minimum add a clarifying comment explaining why 15 is chosen so future changes to RunControl won’t break the access.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/spatial_demo.py`:
- Around line 157-160: The call myResultsScenario.datasheet(...) uses a
non-existent method name; change it to
myResultsScenario.datasheets(name="helloworldSpatial_IntermediateDatasheet",
show_full_paths=True) so it matches the correct plural API used elsewhere (e.g.,
other calls around lines 150–151) and avoids the AttributeError at runtime.
---
Outside diff comments:
In `@docs/conf.py`:
- Around line 138-139: Remove the stale "rasterio" entry from the
autodoc_mock_imports list in the Sphinx config (the autodoc_mock_imports symbol)
so the project no longer mocks or references rasterio; update the
autodoc_mock_imports list to omit "rasterio" and keep the remaining imports
unchanged.
In `@examples/spatial_demo.py`:
- Around line 10-11: The example imports the rasterio module (symbol: rasterio)
but rasterio was removed from package dependencies, so either add it as an
optional dependency named "spatial" (e.g., in setup.py/pyproject extras_require
add "spatial": ["rasterio"]) so users can pip install it via extras, or update
the example (spatial_demo.py) to document the requirement by adding a
top-of-file comment and/or a try/except around "import rasterio" that raises a
clear message instructing users to pip install rasterio; implement one of these
fixes and keep the import symbol rasterio unchanged in the demo.
---
Nitpick comments:
In `@examples/spatial_demo.py`:
- Around line 182-184: Replace the hardcoded index 15 when accessing
spatialRasters to avoid IndexError if RunControl changes: compute the desired
index from the list (e.g., middle or a named selection) using
len(spatialRasters) and store it in a variable (referencing spatialRasters and
the rasterio.open usage) or at minimum add a clarifying comment explaining why
15 is chosen so future changes to RunControl won’t break the access.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 529f3502-7f2b-4811-8415-276e93132a3a
📒 Files selected for processing (3)
docs/conf.pyexamples/spatial_demo.pysetup.py
🚧 Files skipped from review as they are similar to previous changes (1)
- setup.py
2 participants
Summary by CodeRabbit
Chores
Breaking Changes
Examples
Tests
Docs