You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2026/10/22 05:04:53 UTC

[PR] feat(country-map): prototype per-country build scripts as a notebook alternative [superset]

rusackas opened a new pull request, #45607:
URL: https://github.com/apache/superset/pull/45607

   ### SUMMARY
   `Country Map GeoJSON Generator.ipynb` special-cases ~30 countries in one growing notebook, mixing shared helper functions, per-country correction logic, and inline `.plot()` previews. #45602 fixed the worst symptom (the whole file diffing as binary), but a 30-section notebook is still awkward to review a single-country change in, and painful to resolve real conflicts in.
   
   This is a **prototype**, opened as a draft to validate the idea before committing to converting the rest: per-country Python scripts instead of notebook cells, a couple of small shared modules, and an orchestrator CLI.
   
   **Converts 4 of the ~26 special-cased countries** (not all of them - see Scope below), picked to cover distinct patterns rather than the easy ones:
   - `morocco` - external source pinned to a commit, with a regex ISO-code fix and a validation pass
   - `italy` - one notebook section that produces *two* shipped outputs (`italy_regions`, `italy_regions_and_autonomous_provinces`)
   - `latvia` - an external source that can legitimately fail to load. Deliberately changed from the notebook's behavior: the notebook prints a warning and silently falls back to uncorrected data on a failed fetch; this raises instead, so a broken source actually fails the build (matching #45588's "fail the CI job on a notebook/build error" behavior instead of undermining it)
   - `usa` - the classic "flying islands" case (Hawaii/Alaska repositioned), exercising the one shared geometry helper ported so far (`reposition`)
   
   ### Layout
   ```
   superset-frontend/plugins/plugin-chart-country-map/scripts/
     build_maps.py              # orchestrator CLI
     maps/
       context.py                # BuildContext (Natural Earth data), plot styles, aliases
       data.py                    # Natural Earth download + load
       helpers.py                  # shared geometry helpers (only `reposition` so far)
       output.py                    # simplify + save_geojson, ported from the notebook's own logic
       preview.py                    # --preview: per-country PNG thumbnail + a contact-sheet grid
       countries/
         morocco.py, italy.py, latvia.py, usa.py
   ```
   
   ### Validation
   Ran `build_maps.py --preview` against live Natural Earth data and the pinned community sources. Compared every output to what's currently committed under `src/countries/`:
   
   | output | result |
   |---|---|
   | `morocco.geojson` | byte-for-byte identical |
   | `italy_regions.geojson` | byte-for-byte identical |
   | `italy_regions_and_autonomous_provinces.geojson` | byte-for-byte identical |
   | `latvia.geojson` | byte-for-byte identical |
   | `usa.geojson` | byte-for-byte identical |
   
   `--preview` also renders a working per-country PNG thumbnail and assembles them into a contact-sheet grid image - a visual sanity check a reviewer (or eventually the docs site) can look at without running Python. Alaska/Hawaii visibly show up repositioned near the mainland in the USA thumbnail, confirming the `reposition` helper is wired correctly, not just byte-matching by coincidence.
   
   `mypy`/`ruff`/`ruff-format` pass on the new code (added a `pyproject.toml` per-file `TID251` ignore for this scripts tree, matching the existing top-level `scripts/*` exemption, since this is standalone tooling with no `superset` package import).
   
   ### Scope - what this PR deliberately does NOT do
   - Doesn't convert the other ~22 special-cased countries, or the ~190 "plain" countries that don't need a script at all (just a default Natural Earth lookup).
   - Doesn't write `countries.ts` or `docs/data/countries.json` yet.
   - Doesn't touch or retire the notebook - it stays as the source of truth until (if) this approach is adopted for everything.
   - Doesn't wire this into `update-country-maps.yml` (#45588).
   
   If this pattern looks worth adopting, the follow-up is converting the rest, building the "plain country" default path and the `countries.ts`/docs-json writer, and only then retiring the notebook and repointing #45588 at `build_maps.py`.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   N/A (this PR has no UI surface; see Validation above for the thumbnail/contact-sheet check)
   
   ### TESTING INSTRUCTIONS
   ```
   cd superset-frontend/plugins/plugin-chart-country-map/scripts
   pip install geopandas shapely matplotlib requests
   python3 build_maps.py --preview --out-dir /tmp/maps-out
   # compare /tmp/maps-out/*.geojson against src/countries/*.geojson
   ```
   
   ### ADDITIONAL INFORMATION
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(country-map): retire the notebook in favor of per-country build scripts [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.

codeant-ai-for-open-source[bot] commented on code in PR #45607:
URL: https://github.com/apache/superset/pull/45607#discussion_r4357055321


##########
superset-frontend/plugins/plugin-chart-country-map/scripts/maps/assemble.py:
##########
@@ -0,0 +1,84 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Combines the plain country catalog with the registered per-country
+module overrides into the final set of maps to ship, matching the
+notebook's `get_gdf()` + purge-single-subdivision-countries cells.
+"""
+
+from __future__ import annotations
+
+import geopandas as gpd
+
+from .catalog import COUNTRIES, COUNTRY_NAME_ALIASES
+from .context import BuildContext
+
+
+def get_gdf(
+    ctx: BuildContext, built: dict[str, gpd.GeoDataFrame], name: str
+) -> gpd.GeoDataFrame:
+    """Resolves a single country/map name to its GeoDataFrame: a
+    registered module's output if one exists, otherwise a plain Natural
+    Earth lookup by (aliased) admin name.
+    """
+    if name in built:
+        return built[name].copy()
+
+    alias = COUNTRY_NAME_ALIASES.get(name, name)
+    return ctx.df[ctx.df["admin"].str.lower() == alias].copy()
+
+
+def validate_catalog(ctx: BuildContext, built: dict[str, gpd.GeoDataFrame]) -> None:
+    """Raises if a plain catalog entry doesn't resolve to any Natural
+    Earth rows (and isn't covered by a registered module instead).
+    """
+    admins = set(ctx.df["admin"].str.lower().unique())
+    invalid = [
+        name
+        for name in COUNTRIES
+        if name not in built and COUNTRY_NAME_ALIASES.get(name, name) not in admins
+    ]
+    if invalid:
+        raise ValueError(f"Following country names are not valid: {invalid}")
+
+
+def purged_countries(
+    ctx: BuildContext, built: dict[str, gpd.GeoDataFrame]
+) -> list[str]:
+    """Plain catalog entries with one subdivision or fewer - useless as a
+    choropleth, so dropped from the output (and any stale file for one
+    should be deleted, in case upstream data used to have more).
+    """
+    return [
+        name
+        for name in COUNTRIES
+        if get_gdf(ctx, built, name)["iso_3166_2"].nunique() <= 1
+    ]
+
+
+def resolve_all(
+    ctx: BuildContext, built: dict[str, gpd.GeoDataFrame]
+) -> dict[str, gpd.GeoDataFrame]:
+    """Returns the final {name: GeoDataFrame} map to save: every plain
+    catalog entry with more than one subdivision (others are dropped -
+    they'd be useless as a choropleth), plus every registered module
+    output (region-variant maps included, even though they aren't in
+    the plain catalog at all).
+    """
+    purged = set(purged_countries(ctx, built))
+    kept_plain = [name for name in COUNTRIES if name not in purged]
+    all_names = sorted(set(kept_plain) | set(built.keys()))

Review Comment:
   **Suggestion:** With `--countries`, unbuilt module-managed countries remain in `kept_plain` and are resolved from raw Natural Earth data, overwriting their corrected maps.
   
   **Assessment:** 🟠 `Major` · 🔁 `Occurrence: Often` · 🏷️ `Logic error`
   
   [![Use CodeAnt Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d2d2a49d5e194232b0076f19f5672c8a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=d2d2a49d5e194232b0076f19f5672c8a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/plugins/plugin-chart-country-map/scripts/maps/assemble.py
   **Line:** 83:83
   **Comment:**
   	*Logic Error: With `--countries`, unbuilt module-managed countries remain in `kept_plain` and are resolved from raw Natural Earth data, overwriting their corrected maps.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45607&comment_hash=f8dfa6df8e6c44b55f8ce7f1e19af11503a59778eb85311e93fc4aa8d6dd49f6&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45607&comment_hash=f8dfa6df8e6c44b55f8ce7f1e19af11503a59778eb85311e93fc4aa8d6dd49f6&reaction=dislike'>👎</a>



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(country-map): retire the notebook in favor of per-country build scripts [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.

codeant-ai-for-open-source[bot] commented on code in PR #45607:
URL: https://github.com/apache/superset/pull/45607#discussion_r4357050569


##########
superset-frontend/plugins/plugin-chart-country-map/scripts/build_maps.py:
##########
@@ -0,0 +1,173 @@
+#!/usr/bin/env python3
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Regenerates plugin-chart-country-map's GeoJSON files, countries.ts and
+docs/data/countries.json from upstream Natural Earth + community geo
+data - the script equivalent of running every cell in
+`Country Map GeoJSON Generator.ipynb` top to bottom.
+
+By default writes to the same repo-relative locations the notebook
+always has (../src/countries/, ../src/countries.ts,
+../../../../docs/data/countries.json), so `git diff` after a run shows
+exactly what changed - override --out-dir/--countries-ts-path/
+--docs-json-path to redirect output elsewhere for a non-destructive
+dry run.
+
+Usage:
+    python3 build_maps.py
+    python3 build_maps.py --preview --out-dir /tmp/maps-out \\
+        --countries-ts-path /tmp/maps-out/countries.ts \\
+        --docs-json-path /tmp/maps-out/countries.json
+    python3 build_maps.py --countries france,turkey  # just these modules
+"""
+
+from __future__ import annotations
+
+import argparse
+import json
+import os
+import sys
+
+import geopandas as gpd
+
+_SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__))
+sys.path.insert(0, _SCRIPT_DIR)
+
+from maps.assemble import purged_countries, resolve_all, validate_catalog  # noqa: E402
+from maps.context import BuildContext  # noqa: E402
+from maps.countries import REGISTRY  # noqa: E402
+from maps.data import load_natural_earth  # noqa: E402
+from maps.output import save_geojson  # noqa: E402
+from maps.preview import build_contact_sheet, render_preview  # noqa: E402
+from maps.ts_writer import write_countries_ts, write_docs_json  # noqa: E402
+
+_DEFAULT_OUT_DIR = os.path.join(_SCRIPT_DIR, "..", "src", "countries")
+_DEFAULT_COUNTRIES_TS_PATH = os.path.join(_SCRIPT_DIR, "..", "src", "countries.ts")
+_DEFAULT_DOCS_JSON_PATH = os.path.join(
+    _SCRIPT_DIR, "..", "..", "..", "..", "docs", "data", "countries.json"
+)
+
+
+def _parse_args() -> argparse.Namespace:
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("--data-dir", default=os.path.expanduser("~/Downloads"))
+    parser.add_argument("--out-dir", default=_DEFAULT_OUT_DIR)
+    parser.add_argument("--countries-ts-path", default=_DEFAULT_COUNTRIES_TS_PATH)
+    parser.add_argument("--docs-json-path", default=_DEFAULT_DOCS_JSON_PATH)
+    parser.add_argument(
+        "--countries",
+        help="Comma-separated subset of registered module NAMEs to run "
+        "(default: all registered). The ~190 plain catalog entries that "
+        "don't need a module are always included.",
+    )
+    parser.add_argument(
+        "--preview",
+        action="store_true",
+        help="Also render a PNG thumbnail per output and a contact sheet grid",
+    )
+    parser.add_argument(
+        "--force-download",
+        action="store_true",
+        help="Re-download Natural Earth data even if a same-size cached copy exists",
+    )
+    return parser.parse_args()
+
+
+def _purge_stale_files(
+    ctx: BuildContext, built: dict[str, gpd.GeoDataFrame], out_dir: str
+) -> None:
+    for name in purged_countries(ctx, built):
+        stale_path = os.path.join(out_dir, f"{name.replace(' ', '_')}.geojson")
+        if os.path.exists(stale_path):
+            os.remove(stale_path)
+            print(f"Purged {stale_path}: {name} has only one subdivision")
+
+
+def _save_all(
+    ctx: BuildContext,
+    resolved: dict[str, gpd.GeoDataFrame],
+    out_dir: str,
+    preview: bool,
+) -> list[dict[str, object]]:
+    manifest: list[dict[str, object]] = []
+    for name, gdf in sorted(resolved.items()):
+        path = save_geojson(ctx, name, gdf, out_dir)
+        entry = {
+            "name": name,
+            "geojson": os.path.relpath(path, out_dir),
+            "subdivisions": int(gdf["iso_3166_2"].nunique()),
+        }
+        if preview:
+            thumb_path = render_preview(gdf, name, out_dir)
+            entry["thumbnail"] = os.path.relpath(thumb_path, out_dir)
+        manifest.append(entry)
+    return manifest
+
+
+def main() -> None:
+    args = _parse_args()
+    os.makedirs(args.out_dir, exist_ok=True)
+
+    selected = set(args.countries.split(",")) if args.countries else None
+    modules = [m for m in REGISTRY if selected is None or m.NAME in selected]
+    if not modules:
+        raise SystemExit(f"No registered country modules matched: {args.countries}")
+
+    print(f"Loading Natural Earth data into {args.data_dir}...")
+    ctx = load_natural_earth(args.data_dir, skip_existing=not args.force_download)
+
+    built: dict[str, gpd.GeoDataFrame] = {}
+    for module in modules:
+        print(f"Building {module.NAME}...")
+        built.update(module.build(ctx))
+
+    if selected is None:
+        # Only meaningful once every module has run - a module missing
+        # from a --countries subset would otherwise look "invalid".
+        validate_catalog(ctx, built)
+        _purge_stale_files(ctx, built, args.out_dir)
+
+    resolved = resolve_all(ctx, built)
+    print(f"Resolved {len(resolved)} maps to save")
+
+    manifest = _save_all(ctx, resolved, args.out_dir, args.preview)

Review Comment:
   **Suggestion:** With `--countries` set, `resolve_all` still includes the full catalog, so `_save_all` overwrites unselected corrected country maps with raw Natural Earth data.
   
   **Assessment:** 🟠 `Major` · 🔁 `Occurrence: Often` · 🏷️ `Logic error`
   
   [![Use CodeAnt Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b1a1e7978df24654b1bed4e7902d07ad&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=b1a1e7978df24654b1bed4e7902d07ad&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/plugins/plugin-chart-country-map/scripts/build_maps.py
   **Line:** 147:147
   **Comment:**
   	*Logic Error: With `--countries` set, `resolve_all` still includes the full catalog, so `_save_all` overwrites unselected corrected country maps with raw Natural Earth data.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45607&comment_hash=cbf391f19f32de1f8b3afd31e05e446f4246fe5652d0fff5fb4c1e7c32f8e77d&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45607&comment_hash=cbf391f19f32de1f8b3afd31e05e446f4246fe5652d0fff5fb4c1e7c32f8e77d&reaction=dislike'>👎</a>



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(country-map): retire the notebook in favor of per-country build scripts [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.

sadpandajoe commented on code in PR #45607:
URL: https://github.com/apache/superset/pull/45607#discussion_r4366710709


##########
docs/admin_docs/configuration/country-map-tools.mdx:
##########
@@ -33,8 +33,16 @@ The Country Maps visualization already ships with the maps for the following cou
 To add a new country to the list, you'd have to edit files in
 [@superset-ui/plugin-chart-country-map](https://github.com/apache/superset/tree/master/superset-frontend/plugins/plugin-chart-country-map).
 
-1. Generate a new GeoJSON file for your country following the guide in [this Jupyter notebook](https://github.com/apache/superset/blob/master/superset-frontend/plugins/plugin-chart-country-map/scripts/Country%20Map%20GeoJSON%20Generator.ipynb).
-2. Edit the countries list in [plugin-chart-country-map/src/countries.ts](https://github.com/apache/superset/blob/master/superset-frontend/plugins/plugin-chart-country-map/src/countries.ts).
-3. Install superset-frontend dependencies: `cd superset-frontend && npm install`
-4. Verify your countries in Superset plugins storybook: `npm run plugins:storybook`.
-5. Build and install Superset from source code.
+1. Most countries need no correction at all - check whether yours already resolves via a plain
+   lookup in [`scripts/maps/catalog.py`](https://github.com/apache/superset/blob/master/superset-frontend/plugins/plugin-chart-country-map/scripts/maps/catalog.py)
+   before writing anything.
+2. If it needs a fix (a renamed region, a relocated territory, a replacement data source, ...),
+   add a module under [`scripts/maps/countries/`](https://github.com/apache/superset/tree/master/superset-frontend/plugins/plugin-chart-country-map/scripts/maps/countries) -
+   the existing modules there are the best reference for the pattern. Then import the module and add it to
+   `REGISTRY` in `scripts/maps/countries/__init__.py`; unregistered modules are ignored by the build.
+3. Regenerate the GeoJSON files, `countries.ts` and `docs/data/countries.json`:
+   `cd superset-frontend && npm run update-maps`. Iterate on just your country with a rendered

Review Comment:
   Following step 3 as written on a machine without the Python geo stack fails immediately: `npm run update-maps` shells out to `build_maps.py`, which imports geopandas, and `npm install` (step 4, and only after this step) doesn't provide it, so the contributor gets `ModuleNotFoundError: No module named 'geopandas'`. The plugin README lists `pip install geopandas shapely matplotlib requests`; should that prerequisite be stated here before the regenerate step?



##########
superset-frontend/plugins/plugin-chart-country-map/scripts/build_maps.py:
##########
@@ -0,0 +1,173 @@
+#!/usr/bin/env python3
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Regenerates plugin-chart-country-map's GeoJSON files, countries.ts and
+docs/data/countries.json from upstream Natural Earth + community geo
+data - the script equivalent of running every cell in
+`Country Map GeoJSON Generator.ipynb` top to bottom.
+
+By default writes to the same repo-relative locations the notebook
+always has (../src/countries/, ../src/countries.ts,
+../../../../docs/data/countries.json), so `git diff` after a run shows
+exactly what changed - override --out-dir/--countries-ts-path/
+--docs-json-path to redirect output elsewhere for a non-destructive
+dry run.
+
+Usage:
+    python3 build_maps.py
+    python3 build_maps.py --preview --out-dir /tmp/maps-out \\
+        --countries-ts-path /tmp/maps-out/countries.ts \\
+        --docs-json-path /tmp/maps-out/countries.json
+    python3 build_maps.py --countries france,turkey  # just these modules
+"""
+
+from __future__ import annotations
+
+import argparse
+import json
+import os
+import sys
+
+import geopandas as gpd
+
+_SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__))
+sys.path.insert(0, _SCRIPT_DIR)
+
+from maps.assemble import purged_countries, resolve_all, validate_catalog  # noqa: E402
+from maps.context import BuildContext  # noqa: E402
+from maps.countries import REGISTRY  # noqa: E402
+from maps.data import load_natural_earth  # noqa: E402
+from maps.output import save_geojson  # noqa: E402
+from maps.preview import build_contact_sheet, render_preview  # noqa: E402
+from maps.ts_writer import write_countries_ts, write_docs_json  # noqa: E402
+
+_DEFAULT_OUT_DIR = os.path.join(_SCRIPT_DIR, "..", "src", "countries")
+_DEFAULT_COUNTRIES_TS_PATH = os.path.join(_SCRIPT_DIR, "..", "src", "countries.ts")
+_DEFAULT_DOCS_JSON_PATH = os.path.join(
+    _SCRIPT_DIR, "..", "..", "..", "..", "docs", "data", "countries.json"
+)
+
+
+def _parse_args() -> argparse.Namespace:
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("--data-dir", default=os.path.expanduser("~/Downloads"))
+    parser.add_argument("--out-dir", default=_DEFAULT_OUT_DIR)
+    parser.add_argument("--countries-ts-path", default=_DEFAULT_COUNTRIES_TS_PATH)
+    parser.add_argument("--docs-json-path", default=_DEFAULT_DOCS_JSON_PATH)
+    parser.add_argument(
+        "--countries",
+        help="Comma-separated subset of registered module NAMEs to run "
+        "(default: all registered). The ~190 plain catalog entries that "
+        "don't need a module are always included.",
+    )
+    parser.add_argument(
+        "--preview",
+        action="store_true",
+        help="Also render a PNG thumbnail per output and a contact sheet grid",
+    )
+    parser.add_argument(
+        "--force-download",
+        action="store_true",
+        help="Re-download Natural Earth data even if a same-size cached copy exists",
+    )
+    return parser.parse_args()
+
+
+def _purge_stale_files(
+    ctx: BuildContext, built: dict[str, gpd.GeoDataFrame], out_dir: str
+) -> None:
+    for name in purged_countries(ctx, built):
+        stale_path = os.path.join(out_dir, f"{name.replace(' ', '_')}.geojson")
+        if os.path.exists(stale_path):
+            os.remove(stale_path)
+            print(f"Purged {stale_path}: {name} has only one subdivision")
+
+
+def _save_all(
+    ctx: BuildContext,
+    resolved: dict[str, gpd.GeoDataFrame],
+    out_dir: str,
+    preview: bool,
+) -> list[dict[str, object]]:
+    manifest: list[dict[str, object]] = []
+    for name, gdf in sorted(resolved.items()):
+        path = save_geojson(ctx, name, gdf, out_dir)
+        entry = {
+            "name": name,
+            "geojson": os.path.relpath(path, out_dir),
+            "subdivisions": int(gdf["iso_3166_2"].nunique()),
+        }
+        if preview:
+            thumb_path = render_preview(gdf, name, out_dir)
+            entry["thumbnail"] = os.path.relpath(thumb_path, out_dir)
+        manifest.append(entry)
+    return manifest
+
+
+def main() -> None:
+    args = _parse_args()
+    os.makedirs(args.out_dir, exist_ok=True)
+
+    selected = set(args.countries.split(",")) if args.countries else None
+    modules = [m for m in REGISTRY if selected is None or m.NAME in selected]
+    if not modules:
+        raise SystemExit(f"No registered country modules matched: {args.countries}")
+
+    print(f"Loading Natural Earth data into {args.data_dir}...")
+    ctx = load_natural_earth(args.data_dir, skip_existing=not args.force_download)
+
+    built: dict[str, gpd.GeoDataFrame] = {}
+    for module in modules:
+        print(f"Building {module.NAME}...")
+        built.update(module.build(ctx))
+
+    if selected is None:
+        # Only meaningful once every module has run - a module missing
+        # from a --countries subset would otherwise look "invalid".
+        validate_catalog(ctx, built)
+        _purge_stale_files(ctx, built, args.out_dir)
+
+    resolved = resolve_all(ctx, built, include_plain=selected is None)
+    print(f"Resolved {len(resolved)} maps to save")
+
+    manifest = _save_all(ctx, resolved, args.out_dir, args.preview)
+
+    manifest_path = os.path.join(args.out_dir, "manifest.json")
+    with open(manifest_path, "w", encoding="utf-8") as f:
+        json.dump(manifest, f, indent=2)
+        f.write("\n")
+    print(f"Wrote {len(manifest)} GeoJSON files, manifest at {manifest_path}")
+
+    if selected is None:
+        countries_combined = sorted(resolved.keys())
+        os.makedirs(os.path.dirname(args.countries_ts_path), exist_ok=True)

Review Comment:
   If `--countries-ts-path` or `--docs-json-path` is given as a bare filename (e.g. `countries.ts`), `os.path.dirname()` returns `""` and `os.makedirs("", exist_ok=True)` raises `FileNotFoundError`. The full build then aborts after all the GeoJSON files are written but before `countries.ts` / `docs/data/countries.json`, leaving the outputs out of sync. Same on line 161. Could the directory fall back to `.` (e.g. `os.path.dirname(path) or "."`)?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(country-map): retire the notebook in favor of per-country build scripts [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.

rusackas commented on code in PR #45607:
URL: https://github.com/apache/superset/pull/45607#discussion_r4358021946


##########
superset-frontend/plugins/plugin-chart-country-map/scripts/build_maps.py:
##########
@@ -0,0 +1,173 @@
+#!/usr/bin/env python3
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Regenerates plugin-chart-country-map's GeoJSON files, countries.ts and
+docs/data/countries.json from upstream Natural Earth + community geo
+data - the script equivalent of running every cell in
+`Country Map GeoJSON Generator.ipynb` top to bottom.
+
+By default writes to the same repo-relative locations the notebook
+always has (../src/countries/, ../src/countries.ts,
+../../../../docs/data/countries.json), so `git diff` after a run shows
+exactly what changed - override --out-dir/--countries-ts-path/
+--docs-json-path to redirect output elsewhere for a non-destructive
+dry run.
+
+Usage:
+    python3 build_maps.py
+    python3 build_maps.py --preview --out-dir /tmp/maps-out \\
+        --countries-ts-path /tmp/maps-out/countries.ts \\
+        --docs-json-path /tmp/maps-out/countries.json
+    python3 build_maps.py --countries france,turkey  # just these modules
+"""
+
+from __future__ import annotations
+
+import argparse
+import json
+import os
+import sys
+
+import geopandas as gpd
+
+_SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__))
+sys.path.insert(0, _SCRIPT_DIR)
+
+from maps.assemble import purged_countries, resolve_all, validate_catalog  # noqa: E402
+from maps.context import BuildContext  # noqa: E402
+from maps.countries import REGISTRY  # noqa: E402
+from maps.data import load_natural_earth  # noqa: E402
+from maps.output import save_geojson  # noqa: E402
+from maps.preview import build_contact_sheet, render_preview  # noqa: E402
+from maps.ts_writer import write_countries_ts, write_docs_json  # noqa: E402
+
+_DEFAULT_OUT_DIR = os.path.join(_SCRIPT_DIR, "..", "src", "countries")
+_DEFAULT_COUNTRIES_TS_PATH = os.path.join(_SCRIPT_DIR, "..", "src", "countries.ts")
+_DEFAULT_DOCS_JSON_PATH = os.path.join(
+    _SCRIPT_DIR, "..", "..", "..", "..", "docs", "data", "countries.json"
+)
+
+
+def _parse_args() -> argparse.Namespace:
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("--data-dir", default=os.path.expanduser("~/Downloads"))
+    parser.add_argument("--out-dir", default=_DEFAULT_OUT_DIR)
+    parser.add_argument("--countries-ts-path", default=_DEFAULT_COUNTRIES_TS_PATH)
+    parser.add_argument("--docs-json-path", default=_DEFAULT_DOCS_JSON_PATH)
+    parser.add_argument(
+        "--countries",
+        help="Comma-separated subset of registered module NAMEs to run "
+        "(default: all registered). The ~190 plain catalog entries that "
+        "don't need a module are always included.",
+    )
+    parser.add_argument(
+        "--preview",
+        action="store_true",
+        help="Also render a PNG thumbnail per output and a contact sheet grid",
+    )
+    parser.add_argument(
+        "--force-download",
+        action="store_true",
+        help="Re-download Natural Earth data even if a same-size cached copy exists",
+    )
+    return parser.parse_args()
+
+
+def _purge_stale_files(
+    ctx: BuildContext, built: dict[str, gpd.GeoDataFrame], out_dir: str
+) -> None:
+    for name in purged_countries(ctx, built):
+        stale_path = os.path.join(out_dir, f"{name.replace(' ', '_')}.geojson")
+        if os.path.exists(stale_path):
+            os.remove(stale_path)
+            print(f"Purged {stale_path}: {name} has only one subdivision")
+
+
+def _save_all(
+    ctx: BuildContext,
+    resolved: dict[str, gpd.GeoDataFrame],
+    out_dir: str,
+    preview: bool,
+) -> list[dict[str, object]]:
+    manifest: list[dict[str, object]] = []
+    for name, gdf in sorted(resolved.items()):
+        path = save_geojson(ctx, name, gdf, out_dir)
+        entry = {
+            "name": name,
+            "geojson": os.path.relpath(path, out_dir),
+            "subdivisions": int(gdf["iso_3166_2"].nunique()),
+        }
+        if preview:
+            thumb_path = render_preview(gdf, name, out_dir)
+            entry["thumbnail"] = os.path.relpath(thumb_path, out_dir)
+        manifest.append(entry)
+    return manifest
+
+
+def main() -> None:
+    args = _parse_args()
+    os.makedirs(args.out_dir, exist_ok=True)
+
+    selected = set(args.countries.split(",")) if args.countries else None
+    modules = [m for m in REGISTRY if selected is None or m.NAME in selected]
+    if not modules:
+        raise SystemExit(f"No registered country modules matched: {args.countries}")
+
+    print(f"Loading Natural Earth data into {args.data_dir}...")
+    ctx = load_natural_earth(args.data_dir, skip_existing=not args.force_download)
+
+    built: dict[str, gpd.GeoDataFrame] = {}
+    for module in modules:
+        print(f"Building {module.NAME}...")
+        built.update(module.build(ctx))
+
+    if selected is None:
+        # Only meaningful once every module has run - a module missing
+        # from a --countries subset would otherwise look "invalid".
+        validate_catalog(ctx, built)
+        _purge_stale_files(ctx, built, args.out_dir)
+
+    resolved = resolve_all(ctx, built)
+    print(f"Resolved {len(resolved)} maps to save")
+
+    manifest = _save_all(ctx, resolved, args.out_dir, args.preview)

Review Comment:
   Good catch. `resolve_all` now takes `include_plain`, and a `--countries` subset passes `False`, so only the modules actually built get written.



##########
superset-frontend/plugins/plugin-chart-country-map/scripts/maps/assemble.py:
##########
@@ -0,0 +1,84 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Combines the plain country catalog with the registered per-country
+module overrides into the final set of maps to ship, matching the
+notebook's `get_gdf()` + purge-single-subdivision-countries cells.
+"""
+
+from __future__ import annotations
+
+import geopandas as gpd
+
+from .catalog import COUNTRIES, COUNTRY_NAME_ALIASES
+from .context import BuildContext
+
+
+def get_gdf(
+    ctx: BuildContext, built: dict[str, gpd.GeoDataFrame], name: str
+) -> gpd.GeoDataFrame:
+    """Resolves a single country/map name to its GeoDataFrame: a
+    registered module's output if one exists, otherwise a plain Natural
+    Earth lookup by (aliased) admin name.
+    """
+    if name in built:
+        return built[name].copy()
+
+    alias = COUNTRY_NAME_ALIASES.get(name, name)
+    return ctx.df[ctx.df["admin"].str.lower() == alias].copy()
+
+
+def validate_catalog(ctx: BuildContext, built: dict[str, gpd.GeoDataFrame]) -> None:
+    """Raises if a plain catalog entry doesn't resolve to any Natural
+    Earth rows (and isn't covered by a registered module instead).
+    """
+    admins = set(ctx.df["admin"].str.lower().unique())
+    invalid = [
+        name
+        for name in COUNTRIES
+        if name not in built and COUNTRY_NAME_ALIASES.get(name, name) not in admins
+    ]
+    if invalid:
+        raise ValueError(f"Following country names are not valid: {invalid}")
+
+
+def purged_countries(
+    ctx: BuildContext, built: dict[str, gpd.GeoDataFrame]
+) -> list[str]:
+    """Plain catalog entries with one subdivision or fewer - useless as a
+    choropleth, so dropped from the output (and any stale file for one
+    should be deleted, in case upstream data used to have more).
+    """
+    return [
+        name
+        for name in COUNTRIES
+        if get_gdf(ctx, built, name)["iso_3166_2"].nunique() <= 1
+    ]
+
+
+def resolve_all(
+    ctx: BuildContext, built: dict[str, gpd.GeoDataFrame]
+) -> dict[str, gpd.GeoDataFrame]:
+    """Returns the final {name: GeoDataFrame} map to save: every plain
+    catalog entry with more than one subdivision (others are dropped -
+    they'd be useless as a choropleth), plus every registered module
+    output (region-variant maps included, even though they aren't in
+    the plain catalog at all).
+    """
+    purged = set(purged_countries(ctx, built))
+    kept_plain = [name for name in COUNTRIES if name not in purged]
+    all_names = sorted(set(kept_plain) | set(built.keys()))

Review Comment:
   Same root cause as the `build_maps.py` thread. A subset build now skips the plain catalog entirely, so unbuilt module-managed countries are no longer re-resolved from raw Natural Earth.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(country-map): retire the notebook in favor of per-country build scripts [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.

rusackas commented on code in PR #45607:
URL: https://github.com/apache/superset/pull/45607#discussion_r4367114839


##########
docs/admin_docs/configuration/country-map-tools.mdx:
##########
@@ -33,8 +33,16 @@ The Country Maps visualization already ships with the maps for the following cou
 To add a new country to the list, you'd have to edit files in
 [@superset-ui/plugin-chart-country-map](https://github.com/apache/superset/tree/master/superset-frontend/plugins/plugin-chart-country-map).
 
-1. Generate a new GeoJSON file for your country following the guide in [this Jupyter notebook](https://github.com/apache/superset/blob/master/superset-frontend/plugins/plugin-chart-country-map/scripts/Country%20Map%20GeoJSON%20Generator.ipynb).
-2. Edit the countries list in [plugin-chart-country-map/src/countries.ts](https://github.com/apache/superset/blob/master/superset-frontend/plugins/plugin-chart-country-map/src/countries.ts).
-3. Install superset-frontend dependencies: `cd superset-frontend && npm install`
-4. Verify your countries in Superset plugins storybook: `npm run plugins:storybook`.
-5. Build and install Superset from source code.
+1. Most countries need no correction at all - check whether yours already resolves via a plain
+   lookup in [`scripts/maps/catalog.py`](https://github.com/apache/superset/blob/master/superset-frontend/plugins/plugin-chart-country-map/scripts/maps/catalog.py)
+   before writing anything.
+2. If it needs a fix (a renamed region, a relocated territory, a replacement data source, ...),
+   add a module under [`scripts/maps/countries/`](https://github.com/apache/superset/tree/master/superset-frontend/plugins/plugin-chart-country-map/scripts/maps/countries) -
+   the existing modules there are the best reference for the pattern. Then import the module and add it to
+   `REGISTRY` in `scripts/maps/countries/__init__.py`; unregistered modules are ignored by the build.
+3. Regenerate the GeoJSON files, `countries.ts` and `docs/data/countries.json`:
+   `cd superset-frontend && npm run update-maps`. Iterate on just your country with a rendered

Review Comment:
   Good catch, added the `pip install geopandas shapely matplotlib requests` prerequisite to step 3.



##########
superset-frontend/plugins/plugin-chart-country-map/scripts/build_maps.py:
##########
@@ -0,0 +1,173 @@
+#!/usr/bin/env python3
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Regenerates plugin-chart-country-map's GeoJSON files, countries.ts and
+docs/data/countries.json from upstream Natural Earth + community geo
+data - the script equivalent of running every cell in
+`Country Map GeoJSON Generator.ipynb` top to bottom.
+
+By default writes to the same repo-relative locations the notebook
+always has (../src/countries/, ../src/countries.ts,
+../../../../docs/data/countries.json), so `git diff` after a run shows
+exactly what changed - override --out-dir/--countries-ts-path/
+--docs-json-path to redirect output elsewhere for a non-destructive
+dry run.
+
+Usage:
+    python3 build_maps.py
+    python3 build_maps.py --preview --out-dir /tmp/maps-out \\
+        --countries-ts-path /tmp/maps-out/countries.ts \\
+        --docs-json-path /tmp/maps-out/countries.json
+    python3 build_maps.py --countries france,turkey  # just these modules
+"""
+
+from __future__ import annotations
+
+import argparse
+import json
+import os
+import sys
+
+import geopandas as gpd
+
+_SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__))
+sys.path.insert(0, _SCRIPT_DIR)
+
+from maps.assemble import purged_countries, resolve_all, validate_catalog  # noqa: E402
+from maps.context import BuildContext  # noqa: E402
+from maps.countries import REGISTRY  # noqa: E402
+from maps.data import load_natural_earth  # noqa: E402
+from maps.output import save_geojson  # noqa: E402
+from maps.preview import build_contact_sheet, render_preview  # noqa: E402
+from maps.ts_writer import write_countries_ts, write_docs_json  # noqa: E402
+
+_DEFAULT_OUT_DIR = os.path.join(_SCRIPT_DIR, "..", "src", "countries")
+_DEFAULT_COUNTRIES_TS_PATH = os.path.join(_SCRIPT_DIR, "..", "src", "countries.ts")
+_DEFAULT_DOCS_JSON_PATH = os.path.join(
+    _SCRIPT_DIR, "..", "..", "..", "..", "docs", "data", "countries.json"
+)
+
+
+def _parse_args() -> argparse.Namespace:
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("--data-dir", default=os.path.expanduser("~/Downloads"))
+    parser.add_argument("--out-dir", default=_DEFAULT_OUT_DIR)
+    parser.add_argument("--countries-ts-path", default=_DEFAULT_COUNTRIES_TS_PATH)
+    parser.add_argument("--docs-json-path", default=_DEFAULT_DOCS_JSON_PATH)
+    parser.add_argument(
+        "--countries",
+        help="Comma-separated subset of registered module NAMEs to run "
+        "(default: all registered). The ~190 plain catalog entries that "
+        "don't need a module are always included.",
+    )
+    parser.add_argument(
+        "--preview",
+        action="store_true",
+        help="Also render a PNG thumbnail per output and a contact sheet grid",
+    )
+    parser.add_argument(
+        "--force-download",
+        action="store_true",
+        help="Re-download Natural Earth data even if a same-size cached copy exists",
+    )
+    return parser.parse_args()
+
+
+def _purge_stale_files(
+    ctx: BuildContext, built: dict[str, gpd.GeoDataFrame], out_dir: str
+) -> None:
+    for name in purged_countries(ctx, built):
+        stale_path = os.path.join(out_dir, f"{name.replace(' ', '_')}.geojson")
+        if os.path.exists(stale_path):
+            os.remove(stale_path)
+            print(f"Purged {stale_path}: {name} has only one subdivision")
+
+
+def _save_all(
+    ctx: BuildContext,
+    resolved: dict[str, gpd.GeoDataFrame],
+    out_dir: str,
+    preview: bool,
+) -> list[dict[str, object]]:
+    manifest: list[dict[str, object]] = []
+    for name, gdf in sorted(resolved.items()):
+        path = save_geojson(ctx, name, gdf, out_dir)
+        entry = {
+            "name": name,
+            "geojson": os.path.relpath(path, out_dir),
+            "subdivisions": int(gdf["iso_3166_2"].nunique()),
+        }
+        if preview:
+            thumb_path = render_preview(gdf, name, out_dir)
+            entry["thumbnail"] = os.path.relpath(thumb_path, out_dir)
+        manifest.append(entry)
+    return manifest
+
+
+def main() -> None:
+    args = _parse_args()
+    os.makedirs(args.out_dir, exist_ok=True)
+
+    selected = set(args.countries.split(",")) if args.countries else None
+    modules = [m for m in REGISTRY if selected is None or m.NAME in selected]
+    if not modules:
+        raise SystemExit(f"No registered country modules matched: {args.countries}")
+
+    print(f"Loading Natural Earth data into {args.data_dir}...")
+    ctx = load_natural_earth(args.data_dir, skip_existing=not args.force_download)
+
+    built: dict[str, gpd.GeoDataFrame] = {}
+    for module in modules:
+        print(f"Building {module.NAME}...")
+        built.update(module.build(ctx))
+
+    if selected is None:
+        # Only meaningful once every module has run - a module missing
+        # from a --countries subset would otherwise look "invalid".
+        validate_catalog(ctx, built)
+        _purge_stale_files(ctx, built, args.out_dir)
+
+    resolved = resolve_all(ctx, built, include_plain=selected is None)
+    print(f"Resolved {len(resolved)} maps to save")
+
+    manifest = _save_all(ctx, resolved, args.out_dir, args.preview)
+
+    manifest_path = os.path.join(args.out_dir, "manifest.json")
+    with open(manifest_path, "w", encoding="utf-8") as f:
+        json.dump(manifest, f, indent=2)
+        f.write("\n")
+    print(f"Wrote {len(manifest)} GeoJSON files, manifest at {manifest_path}")
+
+    if selected is None:
+        countries_combined = sorted(resolved.keys())
+        os.makedirs(os.path.dirname(args.countries_ts_path), exist_ok=True)

Review Comment:
   Fixed, both `makedirs` calls now fall back to `.` for bare filenames.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(country-map): retire the notebook in favor of per-country build scripts [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.

rusackas commented on code in PR #45607:
URL: https://github.com/apache/superset/pull/45607#discussion_r4367599054


##########
superset-frontend/plugins/plugin-chart-country-map/scripts/build_maps.py:
##########
@@ -0,0 +1,173 @@
+#!/usr/bin/env python3
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Regenerates plugin-chart-country-map's GeoJSON files, countries.ts and
+docs/data/countries.json from upstream Natural Earth + community geo
+data - the script equivalent of running every cell in
+`Country Map GeoJSON Generator.ipynb` top to bottom.
+
+By default writes to the same repo-relative locations the notebook
+always has (../src/countries/, ../src/countries.ts,
+../../../../docs/data/countries.json), so `git diff` after a run shows
+exactly what changed - override --out-dir/--countries-ts-path/
+--docs-json-path to redirect output elsewhere for a non-destructive
+dry run.
+
+Usage:
+    python3 build_maps.py
+    python3 build_maps.py --preview --out-dir /tmp/maps-out \\
+        --countries-ts-path /tmp/maps-out/countries.ts \\
+        --docs-json-path /tmp/maps-out/countries.json
+    python3 build_maps.py --countries france,turkey  # just these modules
+"""
+
+from __future__ import annotations
+
+import argparse
+import json
+import os
+import sys
+
+import geopandas as gpd
+
+_SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__))
+sys.path.insert(0, _SCRIPT_DIR)
+
+from maps.assemble import purged_countries, resolve_all, validate_catalog  # noqa: E402
+from maps.context import BuildContext  # noqa: E402
+from maps.countries import REGISTRY  # noqa: E402
+from maps.data import load_natural_earth  # noqa: E402
+from maps.output import save_geojson  # noqa: E402
+from maps.preview import build_contact_sheet, render_preview  # noqa: E402
+from maps.ts_writer import write_countries_ts, write_docs_json  # noqa: E402
+
+_DEFAULT_OUT_DIR = os.path.join(_SCRIPT_DIR, "..", "src", "countries")
+_DEFAULT_COUNTRIES_TS_PATH = os.path.join(_SCRIPT_DIR, "..", "src", "countries.ts")
+_DEFAULT_DOCS_JSON_PATH = os.path.join(
+    _SCRIPT_DIR, "..", "..", "..", "..", "docs", "data", "countries.json"
+)
+
+
+def _parse_args() -> argparse.Namespace:
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("--data-dir", default=os.path.expanduser("~/Downloads"))
+    parser.add_argument("--out-dir", default=_DEFAULT_OUT_DIR)
+    parser.add_argument("--countries-ts-path", default=_DEFAULT_COUNTRIES_TS_PATH)
+    parser.add_argument("--docs-json-path", default=_DEFAULT_DOCS_JSON_PATH)
+    parser.add_argument(
+        "--countries",
+        help="Comma-separated subset of registered module NAMEs to run "
+        "(default: all registered). The ~190 plain catalog entries that "
+        "don't need a module are always included.",

Review Comment:
   Good catch. Reworded the `--countries` help so it says the plain entries are only included when the flag is omitted, and that a subset build writes just the named modules.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(country-map): retire the notebook in favor of per-country build scripts [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.

codeant-ai-for-open-source[bot] commented on PR #45607:
URL: https://github.com/apache/superset/pull/45607#issuecomment-6359599974

   ## CodeAnt PR Risk: Medium Risk
   
   - The PR needs attention before merging because subset builds still write maps for countries they did not process.
   - With --countries set, resolve_all includes the full catalog, so unselected countries can have their corrected GeoJSON overwritten with raw Natural Earth data.
   
   **Assessed commit:** `f6777ae485a8`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(country-map): retire the notebook in favor of per-country build scripts [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.

sadpandajoe commented on code in PR #45607:
URL: https://github.com/apache/superset/pull/45607#discussion_r4365906910


##########
docs/admin_docs/configuration/country-map-tools.mdx:
##########
@@ -33,8 +33,15 @@ The Country Maps visualization already ships with the maps for the following cou
 To add a new country to the list, you'd have to edit files in
 [@superset-ui/plugin-chart-country-map](https://github.com/apache/superset/tree/master/superset-frontend/plugins/plugin-chart-country-map).
 
-1. Generate a new GeoJSON file for your country following the guide in [this Jupyter notebook](https://github.com/apache/superset/blob/master/superset-frontend/plugins/plugin-chart-country-map/scripts/Country%20Map%20GeoJSON%20Generator.ipynb).
-2. Edit the countries list in [plugin-chart-country-map/src/countries.ts](https://github.com/apache/superset/blob/master/superset-frontend/plugins/plugin-chart-country-map/src/countries.ts).
-3. Install superset-frontend dependencies: `cd superset-frontend && npm install`
-4. Verify your countries in Superset plugins storybook: `npm run plugins:storybook`.
-5. Build and install Superset from source code.
+1. Most countries need no correction at all - check whether yours already resolves via a plain
+   lookup in [`scripts/maps/catalog.py`](https://github.com/apache/superset/blob/master/superset-frontend/plugins/plugin-chart-country-map/scripts/maps/catalog.py)
+   before writing anything.
+2. If it needs a fix (a renamed region, a relocated territory, a replacement data source, ...),

Review Comment:
   build_maps.py only runs modules listed in REGISTRY in scripts/maps/countries/__init__.py, so a new module added per these steps is silently ignored, and the step 3 `--countries <name>` preview exits with "No registered country modules matched." Should the steps say to import and register the new module there?



##########
superset-frontend/plugins/plugin-chart-country-map/scripts/maps/countries/philippines.py:
##########
@@ -0,0 +1,84 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Philippines: a province-level community source replaces the raw,
+outdated Natural Earth boundaries, plus a dissolve of the (unrelated,
+Natural-Earth-sourced) regions.
+
+The notebook this replaces fetched this same community source, renamed
+its columns, and then never wired the result into its output - the
+plain "philippines" entry it shipped was actually still raw Natural
+Earth data (highly-urbanized cities split out as their own polygons, an
+undivided "Maguindanao", "Compostela Valley" instead of "Davao de
+Oro"). Confirmed against the previously-committed philippines.geojson.
+This fixes that: the corrected 83-province source is now the
+"philippines" output.
+"""
+
+from __future__ import annotations
+
+import geopandas as gpd
+
+from maps.context import BuildContext
+
+NAME = "philippines"
+
+_URL = (
+    "https://raw.githubusercontent.com/jdruii/phgeojson/"
+    "098399b6f27ca10e31c8f304e4148b3a56431ec2/philippines.geojson"
+)
+
+
+def build(ctx: BuildContext) -> dict[str, gpd.GeoDataFrame]:
+    try:
+        source = gpd.read_file(_URL)
+    except Exception as exc:  # noqa: BLE001 - re-raised with context below
+        raise RuntimeError(
+            f"Unable to download or load the GeoJSON file for Philippines: {exc}"
+        ) from exc
+
+    required_columns = {"NAME_1", "ISO", "geometry"}
+    if missing_columns := required_columns - set(source.columns):
+        raise ValueError(
+            f"Philippines source is missing columns: {sorted(missing_columns)}. "
+            f"Available columns: {sorted(source.columns)}"
+        )
+
+    philippines = source.rename(columns={"NAME_1": "name", "ISO": "iso_3166_2"})[

Review Comment:
   The regenerated philippines.geojson has 8 counterclockwise exterior rings across Davao Occidental, Davao del Sur, Dinagat Islands, Maguindanao del Norte and Sarangani (the previous file had none). The chart projects with d3 v3, which treats a counterclockwise exterior as the complement on the sphere, so selecting Philippines can fit bounds to nearly the whole globe and fill outside those provinces. Could the winding be normalized (e.g. shapely.geometry.polygon.orient on each polygon) before saving, and the file regenerated?



##########
superset-frontend/plugins/plugin-chart-country-map/README.md:
##########
@@ -53,10 +53,21 @@ for more details.
 
 ### Update Map
 
-To update the country maps or add a new country, run scripts in the Jupyter notebook
-`scripts/Country Map GeoJSON Generator.ipynb`.
+To regenerate every country's GeoJSON from upstream Natural Earth and
+community sources, `countries.ts`, and `docs/data/countries.json`, run:
 
 ```bash
-pip install geopandas shapely matplotlib notebook
-jupyter notebook
+pip install geopandas shapely matplotlib requests
+npm run update-maps

Review Comment:
   update-maps is defined in superset-frontend/package.json, not in this plugin's package.json, yet the python3 scripts/build_maps.py example below assumes the plugin directory. Run from here, `npm run update-maps` fails with a missing script. Could each command state the directory it runs from?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(country-map): prototype per-country build scripts as a notebook alternative [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.

codecov[bot] commented on PR #45607:
URL: https://github.com/apache/superset/pull/45607#issuecomment-6333415352

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/45607?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 65.27%. Comparing base ([`4e5d0ef`](https://app.codecov.io/gh/apache/superset/commit/4e5d0ef8f13d1b878801851e1801f9672ef155e4?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`25b1a4a`](https://app.codecov.io/gh/apache/superset/commit/25b1a4ae97c4f1dda6b07091d721633cf67157b7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 2 commits behind head on master.
   > :exclamation:  There is a different number of reports uploaded between BASE (4e5d0ef) and HEAD (25b1a4a). Click for more details.
   > 
   > <details><summary>HEAD has 4 uploads less than BASE</summary>
   >
   >| Flag | BASE (4e5d0ef) | HEAD (25b1a4a) |
   >|------|------|------|
   >|python|7|3|
   ></details>
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #45607       +/-   ##
   ===========================================
   - Coverage   84.10%   65.27%   -18.84%     
   ===========================================
     Files        3056     3056               
     Lines      201285   201286        +1     
     Branches    47066    47066               
   ===========================================
   - Hits       169299   131396    -37903     
   - Misses      28747    66933    +38186     
   + Partials     3239     2957      -282     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/45607/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [hive](https://app.codecov.io/gh/apache/superset/pull/45607/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `34.89% <ø> (ø)` | |
   | [javascript](https://app.codecov.io/gh/apache/superset/pull/45607/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `80.15% <ø> (-0.02%)` | :arrow_down: |
   | [mysql](https://app.codecov.io/gh/apache/superset/pull/45607/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [postgres](https://app.codecov.io/gh/apache/superset/pull/45607/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [presto](https://app.codecov.io/gh/apache/superset/pull/45607/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `36.62% <ø> (ø)` | |
   | [python](https://app.codecov.io/gh/apache/superset/pull/45607/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `52.73% <ø> (-34.62%)` | :arrow_down: |
   | [sqlite](https://app.codecov.io/gh/apache/superset/pull/45607/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `52.62% <ø> (ø)` | |
   | [unit](https://app.codecov.io/gh/apache/superset/pull/45607/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/45607?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(country-map): prototype per-country build scripts as a notebook alternative [superset]

Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.

netlify[bot] commented on PR #45607:
URL: https://github.com/apache/superset/pull/45607#issuecomment-6333202655

   ### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
   
   
   |  Name | Link |
   |:-:|------------------------|
   |<span aria-hidden="true">🔨</span> Latest commit | 25b1a4ae97c4f1dda6b07091d721633cf67157b7 |
   |<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6ad99975ee8c380dccca768a |
   |<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-45607--superset-docs-preview.netlify.app](https://deploy-preview-45607--superset-docs-preview.netlify.app) |
   |<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br />![QR Code](https://app.netlify.com/qr-code/eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1cmwiOiJodHRwczovL2RlcGxveS1wcmV2aWV3LTQ1NjA3LS1zdXBlcnNldC1kb2NzLXByZXZpZXcubmV0bGlmeS5hcHAifQ.4WQRRLy54gFFw5MAYII2B_Brm2Xk2IV-O1hUjj3xl_E)<br /><br />_Use your smartphone camera to open QR code link._</details> |
   |<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcoxCoAwDADAv2S2KIi19Qc-IyYVBbUhxqGIf7fbDfeCaD7FYAJoYFG8aKtmxdVays9lWtyJ4m7SXeyuiZMcucxcm0eOMY5DSoH60DER4egDwvcD) |
   ---
   <!-- [superset-docs-preview Preview](https://deploy-preview-45607--superset-docs-preview.netlify.app) -->
   _To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(country-map): retire the notebook in favor of per-country build scripts [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.

sadpandajoe commented on code in PR #45607:
URL: https://github.com/apache/superset/pull/45607#discussion_r4367892464


##########
superset-frontend/plugins/plugin-chart-country-map/scripts/build_maps.py:
##########
@@ -0,0 +1,174 @@
+#!/usr/bin/env python3
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Regenerates plugin-chart-country-map's GeoJSON files, countries.ts and
+docs/data/countries.json from upstream Natural Earth + community geo
+data - the script equivalent of running every cell in
+`Country Map GeoJSON Generator.ipynb` top to bottom.
+
+By default writes to the same repo-relative locations the notebook
+always has (../src/countries/, ../src/countries.ts,
+../../../../docs/data/countries.json), so `git diff` after a run shows
+exactly what changed - override --out-dir/--countries-ts-path/
+--docs-json-path to redirect output elsewhere for a non-destructive
+dry run.
+
+Usage:
+    python3 build_maps.py
+    python3 build_maps.py --preview --out-dir /tmp/maps-out \\
+        --countries-ts-path /tmp/maps-out/countries.ts \\
+        --docs-json-path /tmp/maps-out/countries.json
+    python3 build_maps.py --countries france,turkey  # just these modules
+"""
+
+from __future__ import annotations
+
+import argparse
+import json
+import os
+import sys
+
+import geopandas as gpd
+
+_SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__))
+sys.path.insert(0, _SCRIPT_DIR)
+
+from maps.assemble import purged_countries, resolve_all, validate_catalog  # noqa: E402
+from maps.context import BuildContext  # noqa: E402
+from maps.countries import REGISTRY  # noqa: E402
+from maps.data import load_natural_earth  # noqa: E402
+from maps.output import save_geojson  # noqa: E402
+from maps.preview import build_contact_sheet, render_preview  # noqa: E402
+from maps.ts_writer import write_countries_ts, write_docs_json  # noqa: E402
+
+_DEFAULT_OUT_DIR = os.path.join(_SCRIPT_DIR, "..", "src", "countries")
+_DEFAULT_COUNTRIES_TS_PATH = os.path.join(_SCRIPT_DIR, "..", "src", "countries.ts")
+_DEFAULT_DOCS_JSON_PATH = os.path.join(
+    _SCRIPT_DIR, "..", "..", "..", "..", "docs", "data", "countries.json"
+)
+
+
+def _parse_args() -> argparse.Namespace:
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("--data-dir", default=os.path.expanduser("~/Downloads"))
+    parser.add_argument("--out-dir", default=_DEFAULT_OUT_DIR)
+    parser.add_argument("--countries-ts-path", default=_DEFAULT_COUNTRIES_TS_PATH)
+    parser.add_argument("--docs-json-path", default=_DEFAULT_DOCS_JSON_PATH)
+    parser.add_argument(
+        "--countries",
+        help="Comma-separated subset of registered module NAMEs to run "
+        "(default: all registered). The ~190 plain catalog entries that "
+        "don't need a module are included only when this flag is omitted; "
+        "a subset build writes just the named modules.",
+    )
+    parser.add_argument(
+        "--preview",
+        action="store_true",
+        help="Also render a PNG thumbnail per output and a contact sheet grid",
+    )
+    parser.add_argument(
+        "--force-download",
+        action="store_true",
+        help="Re-download Natural Earth data even if a same-size cached copy exists",
+    )
+    return parser.parse_args()
+
+
+def _purge_stale_files(
+    ctx: BuildContext, built: dict[str, gpd.GeoDataFrame], out_dir: str
+) -> None:
+    for name in purged_countries(ctx, built):
+        stale_path = os.path.join(out_dir, f"{name.replace(' ', '_')}.geojson")
+        if os.path.exists(stale_path):
+            os.remove(stale_path)
+            print(f"Purged {stale_path}: {name} has only one subdivision")
+
+
+def _save_all(
+    ctx: BuildContext,
+    resolved: dict[str, gpd.GeoDataFrame],
+    out_dir: str,
+    preview: bool,
+) -> list[dict[str, object]]:
+    manifest: list[dict[str, object]] = []
+    for name, gdf in sorted(resolved.items()):
+        path = save_geojson(ctx, name, gdf, out_dir)
+        entry = {
+            "name": name,
+            "geojson": os.path.relpath(path, out_dir),
+            "subdivisions": int(gdf["iso_3166_2"].nunique()),
+        }
+        if preview:
+            thumb_path = render_preview(gdf, name, out_dir)
+            entry["thumbnail"] = os.path.relpath(thumb_path, out_dir)
+        manifest.append(entry)
+    return manifest
+
+
+def main() -> None:
+    args = _parse_args()
+    os.makedirs(args.out_dir, exist_ok=True)
+
+    selected = set(args.countries.split(",")) if args.countries else None
+    modules = [m for m in REGISTRY if selected is None or m.NAME in selected]
+    if not modules:
+        raise SystemExit(f"No registered country modules matched: {args.countries}")
+
+    print(f"Loading Natural Earth data into {args.data_dir}...")
+    ctx = load_natural_earth(args.data_dir, skip_existing=not args.force_download)
+
+    built: dict[str, gpd.GeoDataFrame] = {}
+    for module in modules:
+        print(f"Building {module.NAME}...")
+        built.update(module.build(ctx))
+
+    if selected is None:
+        # Only meaningful once every module has run - a module missing
+        # from a --countries subset would otherwise look "invalid".
+        validate_catalog(ctx, built)
+        _purge_stale_files(ctx, built, args.out_dir)
+
+    resolved = resolve_all(ctx, built, include_plain=selected is None)

Review Comment:
   Two regressions fixed during this review, the `--countries` subset rewriting corrected maps from raw Natural Earth data and the bare-filename `makedirs` crash, have no test, and nothing in CI runs these scripts: the existing Jest tests only read the committed artifacts. Reverting `include_plain=selected is None` or dropping an `or "."` fallback would pass every check. Could a small pytest run `main()` against fixtures, once with `--countries france` over an existing corrected `usa.geojson` (asserting it, `countries.ts` and the docs JSON are untouched), and once in a temp cwd with `--countries-ts-path countries.ts`, and be wired into an existing Python CI job?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(country-map): retire the notebook in favor of per-country build scripts [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.

sadpandajoe commented on code in PR #45607:
URL: https://github.com/apache/superset/pull/45607#discussion_r4367149271


##########
superset-frontend/plugins/plugin-chart-country-map/scripts/build_maps.py:
##########
@@ -0,0 +1,173 @@
+#!/usr/bin/env python3
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Regenerates plugin-chart-country-map's GeoJSON files, countries.ts and
+docs/data/countries.json from upstream Natural Earth + community geo
+data - the script equivalent of running every cell in
+`Country Map GeoJSON Generator.ipynb` top to bottom.
+
+By default writes to the same repo-relative locations the notebook
+always has (../src/countries/, ../src/countries.ts,
+../../../../docs/data/countries.json), so `git diff` after a run shows
+exactly what changed - override --out-dir/--countries-ts-path/
+--docs-json-path to redirect output elsewhere for a non-destructive
+dry run.
+
+Usage:
+    python3 build_maps.py
+    python3 build_maps.py --preview --out-dir /tmp/maps-out \\
+        --countries-ts-path /tmp/maps-out/countries.ts \\
+        --docs-json-path /tmp/maps-out/countries.json
+    python3 build_maps.py --countries france,turkey  # just these modules
+"""
+
+from __future__ import annotations
+
+import argparse
+import json
+import os
+import sys
+
+import geopandas as gpd
+
+_SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__))
+sys.path.insert(0, _SCRIPT_DIR)
+
+from maps.assemble import purged_countries, resolve_all, validate_catalog  # noqa: E402
+from maps.context import BuildContext  # noqa: E402
+from maps.countries import REGISTRY  # noqa: E402
+from maps.data import load_natural_earth  # noqa: E402
+from maps.output import save_geojson  # noqa: E402
+from maps.preview import build_contact_sheet, render_preview  # noqa: E402
+from maps.ts_writer import write_countries_ts, write_docs_json  # noqa: E402
+
+_DEFAULT_OUT_DIR = os.path.join(_SCRIPT_DIR, "..", "src", "countries")
+_DEFAULT_COUNTRIES_TS_PATH = os.path.join(_SCRIPT_DIR, "..", "src", "countries.ts")
+_DEFAULT_DOCS_JSON_PATH = os.path.join(
+    _SCRIPT_DIR, "..", "..", "..", "..", "docs", "data", "countries.json"
+)
+
+
+def _parse_args() -> argparse.Namespace:
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("--data-dir", default=os.path.expanduser("~/Downloads"))
+    parser.add_argument("--out-dir", default=_DEFAULT_OUT_DIR)
+    parser.add_argument("--countries-ts-path", default=_DEFAULT_COUNTRIES_TS_PATH)
+    parser.add_argument("--docs-json-path", default=_DEFAULT_DOCS_JSON_PATH)
+    parser.add_argument(
+        "--countries",
+        help="Comma-separated subset of registered module NAMEs to run "
+        "(default: all registered). The ~190 plain catalog entries that "
+        "don't need a module are always included.",

Review Comment:
   The `--countries` help says the plain catalog entries are "always included", but a subset build now skips them (`include_plain=False`), so only the named modules are written. Anyone relying on this help text to refresh the plain maps with `--countries x` gets nothing. Should the help say they are only included when `--countries` is omitted?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(country-map): retire the notebook in favor of per-country build scripts [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.

rusackas commented on code in PR #45607:
URL: https://github.com/apache/superset/pull/45607#discussion_r4366114318


##########
superset-frontend/plugins/plugin-chart-country-map/scripts/maps/countries/philippines.py:
##########
@@ -0,0 +1,84 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Philippines: a province-level community source replaces the raw,
+outdated Natural Earth boundaries, plus a dissolve of the (unrelated,
+Natural-Earth-sourced) regions.
+
+The notebook this replaces fetched this same community source, renamed
+its columns, and then never wired the result into its output - the
+plain "philippines" entry it shipped was actually still raw Natural
+Earth data (highly-urbanized cities split out as their own polygons, an
+undivided "Maguindanao", "Compostela Valley" instead of "Davao de
+Oro"). Confirmed against the previously-committed philippines.geojson.
+This fixes that: the corrected 83-province source is now the
+"philippines" output.
+"""
+
+from __future__ import annotations
+
+import geopandas as gpd
+
+from maps.context import BuildContext
+
+NAME = "philippines"
+
+_URL = (
+    "https://raw.githubusercontent.com/jdruii/phgeojson/"
+    "098399b6f27ca10e31c8f304e4148b3a56431ec2/philippines.geojson"
+)
+
+
+def build(ctx: BuildContext) -> dict[str, gpd.GeoDataFrame]:
+    try:
+        source = gpd.read_file(_URL)
+    except Exception as exc:  # noqa: BLE001 - re-raised with context below
+        raise RuntimeError(
+            f"Unable to download or load the GeoJSON file for Philippines: {exc}"
+        ) from exc
+
+    required_columns = {"NAME_1", "ISO", "geometry"}
+    if missing_columns := required_columns - set(source.columns):
+        raise ValueError(
+            f"Philippines source is missing columns: {sorted(missing_columns)}. "
+            f"Available columns: {sorted(source.columns)}"
+        )
+
+    philippines = source.rename(columns={"NAME_1": "name", "ISO": "iso_3166_2"})[

Review Comment:
   Good catch. Reoriented the 5 affected provinces to clockwise exteriors in `philippines.geojson` and added the same orientation to `philippines.py` so a regen keeps it.



##########
docs/admin_docs/configuration/country-map-tools.mdx:
##########
@@ -33,8 +33,15 @@ The Country Maps visualization already ships with the maps for the following cou
 To add a new country to the list, you'd have to edit files in
 [@superset-ui/plugin-chart-country-map](https://github.com/apache/superset/tree/master/superset-frontend/plugins/plugin-chart-country-map).
 
-1. Generate a new GeoJSON file for your country following the guide in [this Jupyter notebook](https://github.com/apache/superset/blob/master/superset-frontend/plugins/plugin-chart-country-map/scripts/Country%20Map%20GeoJSON%20Generator.ipynb).
-2. Edit the countries list in [plugin-chart-country-map/src/countries.ts](https://github.com/apache/superset/blob/master/superset-frontend/plugins/plugin-chart-country-map/src/countries.ts).
-3. Install superset-frontend dependencies: `cd superset-frontend && npm install`
-4. Verify your countries in Superset plugins storybook: `npm run plugins:storybook`.
-5. Build and install Superset from source code.
+1. Most countries need no correction at all - check whether yours already resolves via a plain
+   lookup in [`scripts/maps/catalog.py`](https://github.com/apache/superset/blob/master/superset-frontend/plugins/plugin-chart-country-map/scripts/maps/catalog.py)
+   before writing anything.
+2. If it needs a fix (a renamed region, a relocated territory, a replacement data source, ...),

Review Comment:
   Yep, the docs now say to add the new module to `REGISTRY` in `scripts/maps/countries/__init__.py`.



##########
superset-frontend/plugins/plugin-chart-country-map/README.md:
##########
@@ -53,10 +53,21 @@ for more details.
 
 ### Update Map
 
-To update the country maps or add a new country, run scripts in the Jupyter notebook
-`scripts/Country Map GeoJSON Generator.ipynb`.
+To regenerate every country's GeoJSON from upstream Natural Earth and
+community sources, `countries.ts`, and `docs/data/countries.json`, run:
 
 ```bash
-pip install geopandas shapely matplotlib notebook
-jupyter notebook
+pip install geopandas shapely matplotlib requests
+npm run update-maps

Review Comment:
   Fixed, each command in the README now says which directory it runs from.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org