MNT: install contextily with the other optional requirements - #1179
Draft
ting-hong-shieh wants to merge 1 commit into
Draft
MNT: install contextily with the other optional requirements#1179ting-hong-shieh wants to merge 1 commit into
ting-hong-shieh wants to merge 1 commit into
Conversation
contextily is declared in the monte-carlo extra in pyproject.toml but not
in requirements-optional.txt, and the Makefile's install target reads the
requirements files. test_monte_carlo_plots_background.py opens with
pytest.importorskip("contextily"), so anyone who sets up with `make
install` skips that file: 18 tests, and 53 statements that Codecov counts
as covered.
The workflow installs .[all], so CI already has it and is unaffected. What
this fixes is the local suite silently disagreeing with CI, with a skip
reason as the only clue.
Same specifier as pyproject.toml, including the 3.14 marker.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1179 +/- ##
========================================
Coverage 84.57% 84.57%
========================================
Files 131 131
Lines 17527 17527
========================================
Hits 14824 14824
Misses 2703 2703 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ting-hong-shieh
marked this pull request as draft
August 16, 2026 19:10
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
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.
The gap
contextilyis declared in themonte-carloextra inpyproject.toml:but not in
requirements-optional.txt, and that is what the documented setup installs:tests/unit/simulation/test_monte_carlo_plots_background.pyopens withpytest.importorskip("contextily"), so the whole file is skipped.What it costs
Measured at
4263fa95d7fe6f63d9593f01e4ff7a088369e195, running the five steps oftest_pytest.yamlin order with--cov-append:contextilyplots/monte_carlo_plots.pyCodecov reports 84.57% for that commit, and 14,824 hits against 2,703 misses. The second row is the one that matches; the first is what a contributor following the Makefile sees.
Testsinstalls.[all], which does resolve the extra on Python 3.10, so CI is unaffected either way. What this fixes is a local run quietly disagreeing with CI by 53 statements, with a skip reason buried in-rsoutput as the only clue.The change
One line, the same specifier as
pyproject.toml, marker included:Verification
Before, in an environment built from the requirements files:
After installing it:
Those 18 do reach the network — they call
contextily.bounds2imgagainst real tile providers rather than stubbing it. That is existing behavior and not changed here, but it is worth knowing that this makes the default local suite contact Esri, OpenStreetMap and CartoDB where it previously did not. Happy to close this instead if the team would rather those tests were stubbed first, or would rather keep them opt-in.Found while re-measuring the baseline for #709; it is why the figures in my first comment there were 53 statements low.