Fix CI lint deps (isort) + ignore .venv variants#67
Conversation
- Normalize obstruction inputs (fraction vs percent) in stability calculation - Make geopandas optional via requirements-geo.txt - Add VS Code tasks to run tests under Python 3.10 when installed
…gnore for virtual environments
There was a problem hiding this comment.
Pull request overview
This PR aims to unblock CI lint installs by correcting the isort version constraint and improving gitignore rules for versioned virtualenv folders. It also includes additional changes that expand scope: making geopandas optional via a new requirements file, normalizing SpeedTest.obstruction input formats, and adding Windows-focused VS Code tasks—plus an accidentally committed .venv312/ environment.
Changes:
- Fix
requirements-dev.txtto use a validisortconstraint and update.gitignoreto ignore.venv*/folders. - Split heavy geospatial dependency (
geopandas) out ofrequirements.txtinto newrequirements-geo.txt. - Normalize
SpeedTest.obstructionwhen provided as a fraction (0–1) vs percent (0–100), and add Windows-only VS Code helper tasks.
Reviewed changes
Copilot reviewed 10 out of 46 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/models/SpeedTest.py |
Normalizes obstruction input (fraction vs percent) before applying stability penalty. |
requirements.txt |
Removes geopandas from base requirements and adds guidance for optional geo stack. |
requirements-geo.txt |
Adds optional heavy geospatial requirements (currently geopandas). |
requirements-dev.txt |
Fixes invalid isort constraint (>=6.0.0 → >=5.12.0). |
.vscode/tasks.json |
Adds Windows/py-launcher tasks for creating/using .venv310 and running pytest. |
.gitignore |
Adds .venv*/ pattern to ignore versioned venv directories. |
.venv312/share/man/man1/ttx.1 |
Virtualenv artifact added to repo (should be removed). |
.venv312/pyvenv.cfg |
Virtualenv artifact containing machine-specific paths (should be removed). |
.venv312/Scripts/flask.exe |
Committed binary from venv (should be removed). |
.venv312/Scripts/deactivate.bat |
Virtualenv artifact added to repo (should be removed). |
.venv312/Scripts/activate.bat |
Virtualenv artifact added to repo (should be removed). |
.venv312/Scripts/activate |
Virtualenv artifact added to repo (should be removed). |
.venv312/Scripts/Activate.ps1 |
Virtualenv artifact added to repo (should be removed). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "label": "Bootstrap: Create .venv310 (py -3.10)", | ||
| "type": "shell", | ||
| "command": "py", | ||
| "args": ["-3.10", "-m", "venv", ".venv310"], | ||
| "options": { | ||
| "cwd": "${workspaceFolder}" | ||
| }, | ||
| "problemMatcher": [] | ||
| }, | ||
| { | ||
| "label": "Setup: Install CI deps (.venv310)", | ||
| "type": "process", | ||
| "command": ".\\.venv310\\Scripts\\python.exe", | ||
| "args": ["-m", "pip", "install", "-U", "pip", "-r", "requirements-ci.txt"], | ||
| "options": { | ||
| "cwd": "${workspaceFolder}" | ||
| }, | ||
| "problemMatcher": [] |
There was a problem hiding this comment.
These tasks are Windows-specific (py launcher and .\\venv310\\Scripts\\python.exe paths). Consider scoping them under VS Code’s windows task override (or adding equivalent linux/osx entries) so the shared tasks.json remains usable in cross-platform dev setups.
| home = C:\Users\User\AppData\Local\Programs\Python\Python312 | ||
| include-system-site-packages = false | ||
| version = 3.12.10 | ||
| executable = C:\Users\User\AppData\Local\Programs\Python\Python312\python.exe | ||
| command = C:\Users\User\AppData\Local\Programs\Python\Python312\python.exe -m venv C:\Users\User\OneDrive\Desktop\RURAL MAPPER\Rural-Connectivity-Mapper-2026\.venv312 |
There was a problem hiding this comment.
The .venv312 virtual environment has been committed into the repo. This includes machine-specific absolute paths (e.g., C:\Users\...) and will massively bloat the repository. Remove the entire .venv312/ directory from the PR and ensure it is untracked (e.g., git rm -r --cached .venv312) so it stays ignored going forward.
| # Some upstream sources provide obstruction as a fraction (0.0-1.0) | ||
| # while others provide a percent (0.0-100.0). Treat values in (0, 1] | ||
| # as a fraction and convert to percent. | ||
| obstruction = float(self.obstruction or 0.0) | ||
| if 0.0 < obstruction <= 1.0: | ||
| obstruction *= 100.0 |
There was a problem hiding this comment.
calculate_stability() now accepts obstruction values as either a fraction (0–1) or a percent (0–100), but the class/parameter documentation still states obstruction is a percentage. Update the docstring/type docs to reflect the accepted input formats to avoid confusing callers.
| obstruction = float(self.obstruction or 0.0) | ||
| if 0.0 < obstruction <= 1.0: | ||
| obstruction *= 100.0 | ||
|
|
||
| obstruction_penalty = obstruction * self.OBSTRUCTION_WEIGHT |
There was a problem hiding this comment.
There are existing unit tests covering obstruction penalties, but none that validate the new fraction-to-percent normalization (e.g., obstruction=0.1 should behave like 10). Add a small test case to lock in the new behavior and prevent regressions.
What
requirements-dev.txtinvalid constraint (isort>=6.0.0->isort>=5.12.0)..gitignoreto ignore versioned venv folders like.venv312/via.venv*/.Why
GitHub Actions
Lintworkflow installsrequirements-dev.txt; the invalidisortconstraint can breakpip installand fail the workflow run.Verification
scripts/run_linting.pypasses (Ruff + mypy).pytest -v testspasses on Python 3.12 after installingrequirements-ci.txt.