Skip to content

Comments

Add contextily, allow 3.13 Tensorflow#38

Merged
mathomp4 merged 5 commits intomainfrom
feature/update-2025Aug29
Oct 2, 2025
Merged

Add contextily, allow 3.13 Tensorflow#38
mathomp4 merged 5 commits intomainfrom
feature/update-2025Aug29

Conversation

@mathomp4
Copy link
Member

@mathomp4 mathomp4 commented Aug 29, 2025

Closes #36
Closes #28

This PR:

  • Add contextily and ecmwf-opendata as new packages
  • Allows Tensorflow to install with Python 3.13
  • Removes the ipython sqlite fix
  • Adds a torch fix

@mathomp4 mathomp4 self-assigned this Aug 29, 2025
@mathomp4 mathomp4 added enhancement New feature or request add package Request to add package labels Aug 29, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 29, 2025

Walkthrough

Bumps Miniforge to 25.3.1-0, moves rasterio to Conda, adds contextily and ecmwf-opendata to explicit installs, removes an sqlite pin, installs TensorFlow unconditionally, updates CHANGELOG, and adds a new PyTorch example test.

Changes

Cohort / File(s) Summary
Installer
install_miniforge.bash
Bumped EXAMPLE_MINI_VERSION to 25.3.1-0; removed sqlite==3.48.0 pin; moved rasterio to Conda install; added contextily and ecmwf-opendata to explicit pip installs; made TensorFlow installation unconditional.
Changelog
CHANGELOG.md
Updated Unreleased section: Miniforge version bump, TensorFlow for Python 3.13 enabled, rasterio moved to Conda, contextily and ecmwf-opendata added to explicit packages, new PyTorch test entry, and sqlite fix removed.
Tests
tests/torch_example.py
Added a PyTorch CPU example that fits a cubic polynomial to sin(x) via manual-gradient descent and prints training loss and final coefficients.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant DevRun as Installer
    participant Miniforge as Miniforge/Conda
    participant Pip as pip
    DevRun->>Miniforge: install Miniforge (25.3.1-0)
    DevRun->>Miniforge: conda install rasterio, other conda pkgs
    DevRun->>Pip: pip install contextily, ecmwf-opendata, tensorflow, other pip pkgs
    Pip-->>DevRun: install results (success/fail)
    Miniforge-->>DevRun: install results (success/fail)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning Alongside adding contextily and enabling TensorFlow for Python 3.13, this pull request also updates the Miniforge version, moves rasterio to Conda, adds ecmwf-opendata, removes the sqlite fix, and introduces a new PyTorch test—all of which fall outside the linked issues’ scope. Please separate or document the additional changes—such as the Miniforge bump, rasterio move, ecmwf-opendata addition, sqlite fix removal, and torch test—or ensure the linked issues are updated to cover these modifications.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly and accurately summarizes the two primary changes—adding contextily and enabling TensorFlow on Python 3.13—aligning with the pull request objectives without unnecessary detail.
Linked Issues Check ✅ Passed The pull request adds the contextily package to the Conda environment as specified in issue #36 and removes the Python version gating to install TensorFlow unconditionally—thereby enabling TensorFlow on Python 3.13 as required by issue #28.
Description Check ✅ Passed The description directly outlines the key changes introduced by this pull request—adding contextily and ecmwf-opendata, enabling TensorFlow for Python 3.13, removing the ipython sqlite fix, and including a torch fix—clearly relating to the modifications in the changeset.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/update-2025Aug29

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b7b34f8 and 723638a.

📒 Files selected for processing (2)
  • CHANGELOG.md (1 hunks)
  • install_miniforge.bash (4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-02T17:54:37.398Z
Learnt from: mathomp4
PR: GMAO-SI-Team/GEOSpyD#38
File: install_miniforge.bash:654-654
Timestamp: 2025-09-02T17:54:37.398Z
Learning: The GEOSpyD project does not support Intel Macs (macOS x86_64) anymore, only supporting Linux and macOS arm64 (Apple Silicon).

Applied to files:

  • install_miniforge.bash
🔇 Additional comments (11)
install_miniforge.bash (5)

76-76: LGTM: Version bump to 25.3.1-0.

The Miniforge version update is straightforward and aligns with the changelog entry.


606-606: LGTM: Installing rasterio and contextily via conda.

This correctly installs both packages through conda to maintain GDAL ABI consistency, addressing the geospatial dependency stack requirements noted in previous reviews.


650-650: LGTM: rasterio removed from pip install.

Correctly removes rasterio from the pip install sequence since it's now installed via conda (line 606), maintaining consistency across the script.


667-667: LGTM: ecmwf-opendata added.

The addition of ecmwf-opendata via pip is straightforward and aligns with the changelog entry.


654-654: Unconditional TensorFlow install is safe for Python 3.13. TensorFlow 2.20.0 publishes cp313 wheels for Linux x86_64/aarch64 and macOS arm64; pinning to >=2.20.0 for a minimum version is optional.

CHANGELOG.md (6)

14-14: LGTM: Version documentation consistent.

The Miniforge version in the changelog matches the script update to 25.3.1-0.


19-22: LGTM: Changes documented appropriately.

The TensorFlow Python 3.13 enablement and rasterio migration to conda are both documented with clear rationale. The contextily dependency explanation for moving rasterio is particularly helpful.


26-27: LGTM: contextily addition documented.

The addition of contextily as a conda package is correctly documented and matches the implementation.


31-31: LGTM: ecmwf-opendata documented.

The addition of ecmwf-opendata via pip is properly documented and matches the script changes.


32-32: Pytorch test addition noted.

The changelog documents a new pytorch test, which aligns with the PR objectives mentioning a torch-related fix.


36-37: LGTM: sqlite fix removal documented.

The removal of the sqlite version pin is documented with appropriate context, consistent with the PR objectives and indicating the ipython issue has been resolved.


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.

❤️ Share
🧪 Early access (Sonnet 4.5): enabled

We are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience.

Note:

  • Public repositories are always opted into early access features.
  • You can enable or disable early access features from the CodeRabbit UI or by updating the CodeRabbit configuration file.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (5)
CHANGELOG.md (2)

29-29: Terminology: it’s an example, not a test.

CHANGELOG says “Add new pytorch test,” but the file is a runnable example (no assertions) named torch_example.py. Suggest “Add new PyTorch example.”


33-34: Polish phrasing and capitalization.

Suggest: “Remove SQLite pin; IPython no longer needs it.”

tests/torch_example.py (3)

7-9: Make example deterministic.

Set a manual seed for reproducibility so logs don’t vary across runs.

 dtype = torch.float
 device = torch.device("cpu")
 # device = torch.device("cuda:0") # Uncomment this to run on GPU
+torch.manual_seed(0)

21-43: Safer parameter updates.

Wrap updates in torch.no_grad() to avoid autograd bookkeeping if someone later flips requires_grad.

-for t in range(2000):
+for t in range(2000):
     # Forward pass: compute predicted y
     y_pred = a + b * x + c * x ** 2 + d * x ** 3
@@
-    # Update weights using gradient descent
-    a -= learning_rate * grad_a
-    b -= learning_rate * grad_b
-    c -= learning_rate * grad_c
-    d -= learning_rate * grad_d
+    # Update weights using gradient descent
+    with torch.no_grad():
+        a -= learning_rate * grad_a
+        b -= learning_rate * grad_b
+        c -= learning_rate * grad_c
+        d -= learning_rate * grad_d

1-46: Optional: convert to a pytest test or guard execution.

If this should be a test, add assertions (e.g., loss decreases) and rename to test_torch_example.py. If it’s an example, add a main guard so import doesn’t run it.

Example guard:

-# -*- coding: utf-8 -*-
+# -*- coding: utf-8 -*-
@@
-print(f'Result: y = {a.item()} + {b.item()} x + {c.item()} x^2 + {d.item()} x^3')
+if __name__ == "__main__":
+    print(f'Result: y = {a.item()} + {b.item()} x + {c.item()} x^2 + {d.item()} x^3')
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 28cd055 and d2a8894.

📒 Files selected for processing (3)
  • CHANGELOG.md (1 hunks)
  • install_miniforge.bash (3 hunks)
  • tests/torch_example.py (1 hunks)
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md

[grammar] ~18-~18: There might be a mistake here.
Context: ...- Rename always_circular_stereo.py to cartopy_example.py - Enable TensorFlow installation for Pytho...

(QB_NEW_EN)


[grammar] ~24-~24: There might be a mistake here.
Context: ...13 ### Added - Explicit Conda Packages - contextily - Explicit Pip Packages - n...

(QB_NEW_EN)


[grammar] ~25-~25: There might be a mistake here.
Context: ...- Explicit Conda Packages - contextily - Explicit Pip Packages - nco - cdo - ...

(QB_NEW_EN)


[grammar] ~26-~26: There might be a mistake here.
Context: ...s - contextily - Explicit Pip Packages - nco - cdo - Add new pytorch test ### ...

(QB_NEW_EN)


[grammar] ~27-~27: There might be a mistake here.
Context: ...ntextily - Explicit Pip Packages - nco - cdo - Add new pytorch test ### Removed ...

(QB_NEW_EN)


[grammar] ~28-~28: There might be a mistake here.
Context: ... - Explicit Pip Packages - nco - cdo - Add new pytorch test ### Removed - Rem...

(QB_NEW_EN)

🔇 Additional comments (2)
CHANGELOG.md (1)

19-20: Clarify TF Py3.13 support and platform nuance.

TensorFlow now publishes cp313 wheels (Linux x86_64/aarch64, macOS arm64, Windows), so enabling on Python 3.13 is correct. Note macOS x86_64 remains unsupported (last x86_64 macOS was 2.16). Suggest noting “where wheels are available.”

Suggested wording:

  • “Enable TensorFlow installation for Python 3.13 (wheels now available on PyPI for Linux x86_64/aarch64, macOS arm64, and Windows).”
    Sources: PyPI file listings and install docs. (pypi.org, tensorflow.org)
install_miniforge.bash (1)

76-76: Align example Miniforge version with changelog and upstream.

Either bump this to 25.3.1-0 in CHANGELOG (preferred) or adjust here if you intend 25.3.1-3. Today’s latest is 25.3.1-0.

Apply (only if you intend to reflect -3 everywhere, otherwise keep -0 and adjust CHANGELOG):

-EXAMPLE_MINI_VERSION="25.3.1-0"
+EXAMPLE_MINI_VERSION="25.3.1-3"

Upstream release info. (newreleases.io)

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
CHANGELOG.md (1)

24-25: Add rasterio via conda to avoid GDAL ABI mismatches; fix list punctuation.

contextily depends on rasterio; mixing conda GDAL with pip rasterio is fragile. Install rasterio from conda and drop any pip rasterio. Also add colons after the list headers for readability.

Suggested edits:

- Explicit Conda Packages
-  - contextily
+ Explicit Conda Packages:
+  - contextily
+  - rasterio
- Explicit Pip Packages
+ Explicit Pip Packages:

Repo checks:

#!/bin/bash
# Ensure we don't install rasterio via pip anywhere
rg -n -i 'pip\s+install\s+.*rasterio' || true
# Ensure we do install rasterio via conda/mamba/micromamba
rg -n -i '(conda|mamba|micromamba).*(install|create).*-y.*rasterio' || true
🧹 Nitpick comments (1)
CHANGELOG.md (1)

29-29: Capitalize brand: PyTorch.

Minor wording fix.

- Add new pytorch test
+ Add new PyTorch test
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between d2a8894 and 920416d.

📒 Files selected for processing (1)
  • CHANGELOG.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md

[grammar] ~18-~18: There might be a mistake here.
Context: ...- Rename always_circular_stereo.py to cartopy_example.py - Enable TensorFlow installation for Pytho...

(QB_NEW_EN)


[grammar] ~24-~24: There might be a mistake here.
Context: ...13 ### Added - Explicit Conda Packages - contextily - Explicit Pip Packages - n...

(QB_NEW_EN)


[grammar] ~25-~25: There might be a mistake here.
Context: ...- Explicit Conda Packages - contextily - Explicit Pip Packages - nco - cdo - ...

(QB_NEW_EN)


[grammar] ~26-~26: There might be a mistake here.
Context: ...s - contextily - Explicit Pip Packages - nco - cdo - Add new pytorch test ### ...

(QB_NEW_EN)


[grammar] ~27-~27: There might be a mistake here.
Context: ...ntextily - Explicit Pip Packages - nco - cdo - Add new pytorch test ### Removed ...

(QB_NEW_EN)


[grammar] ~28-~28: There might be a mistake here.
Context: ... - Explicit Pip Packages - nco - cdo - Add new pytorch test ### Removed - Rem...

(QB_NEW_EN)

🔇 Additional comments (2)
CHANGELOG.md (2)

14-14: Miniforge version note now matches installer.

This addresses the earlier mismatch; good to go.


33-34: Verify sqlite pin removal and document fallback

  • CHANGELOG.md: remove the leftover “Pin sqlite to version 3.48.0” entry at line 52.
  • CHANGELOG.md (lines 33–34): add a troubleshooting note under the removal entry explaining how to re-pin sqlite to 3.48.0 if ipython regressions reappear.
  • Confirm CI runs ipython startup tests on Linux and macOS without any sqlite pin.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
install_miniforge.bash (1)

554-554: Critical: quoting bug breaks hvplot pin.

The current token reads as hvplot">=0.11.0". It should be hvplot>=0.11.0 (quoted or unquoted). This line will fail package resolution.

-$PACKAGE_INSTALL movingpandas geoviews hvplot">=0.11.0" geopandas bokeh
+$PACKAGE_INSTALL movingpandas geoviews "hvplot>=0.11.0" geopandas bokeh
♻️ Duplicate comments (3)
CHANGELOG.md (1)

19-22: Qualify TensorFlow 3.13 support with minimum version and date.

Please specify the minimum TF version that adds cp313 wheels (2.20.0, released Aug 13, 2025) and that wheels exist for Linux x86_64/aarch64, macOS arm64, and Windows. This prevents accidental installs of older TF on 3.13.

Apply:

-- Enable TensorFlow installation for Python 3.13
-  - This is now possible as TensorFlow has been updated to support Python 3.13
+- Enable TensorFlow installation for Python 3.13
+  - TensorFlow 2.20.0+ (released 2025-08-13) provides CPython 3.13 wheels
+    for Linux x86_64/aarch64, macOS arm64, and Windows.

References: PyPI release 2.20.0 (includes cp313 wheels and classifier), TF pip install guide. (pypi.org, tensorflow.org)

install_miniforge.bash (2)

606-606: Good: install rasterio with contextily via conda.

This avoids mixing pip rasterio wheels with conda-forge GDAL/PROJ.


654-654: Pin TensorFlow to the first version with Python 3.13 wheels.

Unpinned “tensorflow” may resolve to versions without cp313 support on some mirrors. Pin to >=2.20.0.

-$PIP_INSTALL tensorflow evidential-deep-learning silence_tensorflow
+$PIP_INSTALL "tensorflow>=2.20.0" evidential-deep-learning silence_tensorflow

TF 2.20.0 (released 2025-08-13) publishes cp313 wheels for Linux x86_64/aarch64, macOS arm64, and Windows. (pypi.org)

🧹 Nitpick comments (4)
CHANGELOG.md (3)

26-27: Minor formatting: make the subsection header explicit.

Add a colon for consistency with other sections.

-- Explicit Conda Packages
+- Explicit Conda Packages:

31-31: Capitalize “PyTorch”.

Proper noun.

-- Add new pytorch test
+- Add new PyTorch test

35-35: Capitalize “SQLite” and “IPython”.

Editorial polish.

-- Removed `sqlite` fix as it doesn't seem needed anymore for `ipython`
+- Removed `SQLite` fix as it doesn't seem needed anymore for `IPython`
install_miniforge.bash (1)

650-650: Optional: move h5py to conda for ABI consistency.

Pip wheels for h5py bundle their own HDF5 and can clash with conda-forge netcdf/HDF stacks. Consider installing h5py via conda and dropping it from pip.

Apply within this line:

-$PIP_INSTALL PyRTF3 pipenv pymp-pypi h5py
+$PIP_INSTALL PyRTF3 pipenv pymp-pypi

And add earlier near other conda installs (any suitable group):

$PACKAGE_INSTALL h5py
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 920416d and b7b34f8.

📒 Files selected for processing (2)
  • CHANGELOG.md (1 hunks)
  • install_miniforge.bash (3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-02T17:54:37.361Z
Learnt from: mathomp4
PR: GMAO-SI-Team/GEOSpyD#38
File: install_miniforge.bash:654-654
Timestamp: 2025-09-02T17:54:37.361Z
Learning: The GEOSpyD project does not support Intel Macs (macOS x86_64) anymore, only supporting Linux and macOS arm64 (Apple Silicon).

Applied to files:

  • install_miniforge.bash
🪛 LanguageTool
CHANGELOG.md

[grammar] ~18-~18: There might be a mistake here.
Context: ...- Rename always_circular_stereo.py to cartopy_example.py - Enable TensorFlow installation for Pytho...

(QB_NEW_EN)


[grammar] ~26-~26: There might be a mistake here.
Context: ...er ### Added - Explicit Conda Packages - contextily - Explicit Pip Packages - n...

(QB_NEW_EN)


[grammar] ~27-~27: There might be a mistake here.
Context: ...- Explicit Conda Packages - contextily - Explicit Pip Packages - nco - cdo - ...

(QB_NEW_EN)


[grammar] ~28-~28: There might be a mistake here.
Context: ...s - contextily - Explicit Pip Packages - nco - cdo - Add new pytorch test ### ...

(QB_NEW_EN)


[grammar] ~29-~29: There might be a mistake here.
Context: ...ntextily - Explicit Pip Packages - nco - cdo - Add new pytorch test ### Removed ...

(QB_NEW_EN)


[grammar] ~30-~30: There might be a mistake here.
Context: ... - Explicit Pip Packages - nco - cdo - Add new pytorch test ### Removed - Rem...

(QB_NEW_EN)

🔇 Additional comments (2)
CHANGELOG.md (1)

14-14: Version bump matches script — good catch.

Changelog now aligns with EXAMPLE_MINI_VERSION=25.3.1-0.

install_miniforge.bash (1)

76-76: Miniforge example version update looks correct.

Matches the changelog entry (25.3.1-0).

@mathomp4 mathomp4 merged commit a551c27 into main Oct 2, 2025
2 checks passed
@mathomp4 mathomp4 deleted the feature/update-2025Aug29 branch October 2, 2025 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

add package Request to add package enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add contextily Tensorflow does not support 3.13...yet!

1 participant