Skip to content

Commit 4489935

Browse files
authored
fix: multiple fixes (#285)
* update the model registry generation tool to work with local files or release assets * add an experimental warning to the programs API, for now * trim developer docs, update user-facing docs * finish incomplete cache clear functionality
1 parent a593a14 commit 4489935

13 files changed

Lines changed: 470 additions & 386 deletions

File tree

.github/workflows/ci.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,8 @@ jobs:
117117
env:
118118
REPOS_PATH: ${{ github.workspace }}
119119
GITHUB_TOKEN: ${{ github.token }}
120-
TEST_REPO: wpbonelli/modflow6-testmodels
121-
TEST_REF: registry
120+
TEST_REPO: MODFLOW-ORG/modflow6-testmodels
121+
TEST_REF: develop
122122
TEST_SOURCE: modflow6-testmodels
123123
TEST_SOURCE_NAME: mf6/test
124124
MODFLOW_DEVTOOLS_NO_AUTO_SYNC: 1

autotest/test_models.py

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,35 @@ def test_cli_list_with_cache(self, capsys):
494494
assert f"{TEST_SOURCE_NAME}@{TEST_REF}" in captured.out
495495
assert "Models:" in captured.out
496496

497+
def test_cli_clear(self, capsys):
498+
"""Test 'clear' command."""
499+
# Sync a registry first
500+
_DEFAULT_CACHE.clear(source=TEST_SOURCE_NAME, ref=TEST_REF)
501+
source = ModelSourceRepo(
502+
repo=TEST_REPO,
503+
name=TEST_SOURCE_NAME,
504+
refs=[TEST_REF],
505+
)
506+
result = source.sync(ref=TEST_REF)
507+
assert len(result.synced) == 1
508+
509+
# Verify it's cached
510+
assert _DEFAULT_CACHE.has(TEST_SOURCE_NAME, TEST_REF)
511+
512+
# Clear with force flag
513+
import argparse
514+
515+
from modflow_devtools.models.__main__ import cmd_clear
516+
517+
args = argparse.Namespace(source=TEST_SOURCE_NAME, ref=TEST_REF, force=True)
518+
cmd_clear(args)
519+
520+
# Verify it was cleared
521+
assert not _DEFAULT_CACHE.has(TEST_SOURCE_NAME, TEST_REF)
522+
523+
captured = capsys.readouterr()
524+
assert "Cleared 1 cached registry" in captured.out
525+
497526

498527
@pytest.mark.xdist_group("registry_cache")
499528
class TestIntegration:
@@ -550,8 +579,11 @@ def test_sync_and_list_models(self):
550579
class TestMakeRegistry:
551580
"""Test registry creation tool (make_registry.py)."""
552581

553-
def _get_constructed_url(self, mode, repo, ref, **kwargs):
554-
"""Helper to extract constructed URL from make_registry verbose output."""
582+
def _get_constructed_url(self, repo, ref, **kwargs):
583+
"""Helper to extract constructed URL from make_registry verbose output.
584+
585+
Mode is now inferred from presence of asset_file in kwargs.
586+
"""
555587
import tempfile
556588

557589
# Create a temporary directory to use as dummy path
@@ -561,8 +593,6 @@ def _get_constructed_url(self, mode, repo, ref, **kwargs):
561593
sys.executable,
562594
"-m",
563595
"modflow_devtools.models.make_registry",
564-
"--mode",
565-
mode,
566596
"--repo",
567597
repo,
568598
"--ref",
@@ -587,7 +617,6 @@ def _get_constructed_url(self, mode, repo, ref, **kwargs):
587617
def test_url_construction_version(self):
588618
"""Test URL construction for version mode (auto-detects path from directory)."""
589619
url = self._get_constructed_url(
590-
mode="version",
591620
repo="MODFLOW-ORG/modflow6-testmodels",
592621
ref="master",
593622
name="mf6/test",
@@ -600,7 +629,6 @@ def test_url_construction_version(self):
600629
def test_url_construction_version_different_ref(self):
601630
"""Test URL construction for version mode with different ref."""
602631
url = self._get_constructed_url(
603-
mode="version",
604632
repo="MODFLOW-ORG/modflow6-largetestmodels",
605633
ref="develop",
606634
name="mf6/large",
@@ -612,7 +640,6 @@ def test_url_construction_version_different_ref(self):
612640
def test_url_construction_release(self):
613641
"""Test URL construction for release mode."""
614642
url = self._get_constructed_url(
615-
mode="release",
616643
repo="MODFLOW-ORG/modflow6-examples",
617644
ref="current",
618645
asset_file="mf6examples.zip",
@@ -626,7 +653,6 @@ def test_url_construction_release(self):
626653
def test_url_construction_release_custom(self):
627654
"""Test URL construction for release mode with custom repo/tag."""
628655
url = self._get_constructed_url(
629-
mode="release",
630656
repo="username/my-models",
631657
ref="v1.0.0",
632658
asset_file="models.zip",

autotest/test_programs.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import warnings
12
from pathlib import Path
23

34
import pytest
@@ -13,6 +14,9 @@
1314
get_user_config_path,
1415
)
1516

17+
# Suppress experimental API warning for tests
18+
warnings.filterwarnings("ignore", message=".*modflow_devtools.programs.*experimental.*")
19+
1620

1721
class TestProgramCache:
1822
"""Test cache management."""

docs/md/dev/models.md

Lines changed: 31 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -30,20 +30,14 @@ This is a living document which will be updated as development proceeds. As the
3030
- [Model Addressing](#model-addressing)
3131
- [Registry classes](#registry-classes)
3232
- [Module-Level API](#module-level-api)
33-
- [Migration path](#migration-path)
34-
- [Implementation Summary](#implementation-summary)
35-
- [✅ Core Infrastructure](#-core-infrastructure)
36-
- [✅ Registry Classes](#-registry-classes)
37-
- [✅ User Features](#-user-features)
38-
- [✅ Testing](#-testing)
39-
- [🚧 Future Work (Upstream CI)](#-future-work-upstream-ci)
33+
- [Status and Next Steps](#status-and-next-steps)
4034
- [Cross-API Consistency](#cross-api-consistency)
4135
- [Shared Patterns](#shared-patterns)
4236
- [Key Differences](#key-differences)
4337
- [Demo & Usage Examples](#demo--usage-examples)
4438
- [Python API](#python-api)
4539
- [Basic Workflow](#basic-workflow)
46-
- [Convenience Methods on BootstrapSource](#convenience-methods-on-bootstrapsource)
40+
- [Object-Oriented API](#object-oriented-api)
4741
- [Cache Management](#cache-management)
4842
- [CLI Usage](#cli-usage)
4943
- [Show Registry Status](#show-registry-status)
@@ -388,7 +382,6 @@ Required steps in source model repositories include:
388382
```bash
389383
# Downloads from remote and indexes subdirectory (recommended)
390384
python -m modflow_devtools.models.make_registry \
391-
--mode version \
392385
--repo MODFLOW-ORG/modflow6-testmodels \
393386
--ref master \
394387
--name mf6/test \
@@ -398,14 +391,12 @@ Required steps in source model repositories include:
398391

399392
**For release asset models** (zip published with releases):
400393
```bash
401-
# Downloads from remote and indexes subdirectory (recommended)
394+
# Downloads from remote (provide --asset-file to index from release asset)
402395
python -m modflow_devtools.models.make_registry \
403-
--mode release \
404396
--repo MODFLOW-ORG/modflow6-examples \
405397
--ref current \
406398
--asset-file mf6examples.zip \
407399
--name mf6/example \
408-
--path examples \
409400
--output .registry
410401
```
411402

@@ -509,59 +500,11 @@ Expose as `DEFAULT_REGISTRY` a `MergedRegistry` with all sources configured in t
509500

510501
This will break any code checking `isinstance(DEFAULT_REGISTRY, PoochRegistry)`, but it's unlikely anyone is doing that.
511502

512-
## Migration path
513-
514-
The transition to the dynamic registry system is complete. The package no longer ships large TOML registry files - only a minimal bootstrap file (`modflow_devtools/models/models.toml`) that tells the system where to find remote model repositories.
515-
516-
On first import, `modflow-devtools` attempts to auto-sync the default registries. If this fails (e.g., no network), users will get a clear error message when trying to use the registry, directing them to run `python -m modflow_devtools.models sync`.
517-
518-
Since `modflow-devtools` is currently used only internally (dogfooding), there are no external consumers to worry about for backwards compatibility.
519-
520-
### Implementation Summary
521-
522-
The dynamic registry system has been fully implemented with a streamlined object-oriented design:
523-
524-
#### ✅ Core Infrastructure
525-
- **Consolidated implementation**: All code in single `modflow_devtools/models/__init__.py` file
526-
- Bootstrap metadata file (`modflow_devtools/models/models.toml`)
527-
- Registry schema with Pydantic validation
528-
- Cache management via `ModelCache` class
529-
- Sync functionality via `ModelSourceRepo.sync()` method
530-
- Registry discovery via `ModelSourceRepo.discover()` method
531-
- CLI subcommands (`python -m modflow_devtools.models` - sync, info, list)
532-
- **Remote-first registry generation**: `make_registry` downloads from GitHub by default
533-
- **Intelligent path parameter**: Single `--path` parameter auto-detects local vs. remote subpath usage
534-
535-
#### ✅ Registry Classes (Object-Oriented Design)
536-
- `ModelRegistry`: Pydantic data model (files/models/examples)
537-
- `ModelCache`: Cache management with save/load/list/clear
538-
- `ModelSourceRepo`: Source repository with discovery and sync methods
539-
- `ModelSourceConfig`: Configuration container managing multiple sources
540-
- `PoochRegistry`: Remote model fetching with Pooch integration
541-
- `DiscoveredModelRegistry`: Discovery result with metadata
542-
- **No separate modules**: Schema, cache, discovery, and sync logic integrated into classes
543-
- **Method-based API**: Operations are methods on objects (e.g., `source.sync()`, `cache.load()`)
544-
- Auto-sync on first import (`_try_best_effort_sync()`)
545-
546-
#### ✅ User Features
547-
- User config overlay (`~/.config/modflow-devtools/models.toml`)
548-
- Bootstrap config merging via `ModelSourceConfig.load()`
549-
- Explicit model versioning via git refs
550-
- Platform-appropriate cache locations
551-
- Clear error messages when sync is needed
552-
- **Consolidated registry format**: Single `models.toml` file only
553-
554-
#### ✅ Testing
555-
- Comprehensive test suite in `autotest/test_models.py`
556-
- All 34 tests passing
557-
- Tests configured via `.env` file
558-
- Parallel execution with pytest-xdist (`--dist loadgroup`)
559-
- Full mypy type checking with no errors
560-
561-
#### 🚧 Future Work (Upstream CI)
562-
- Add `.github/workflows/registry.yml` to each model repo
563-
- Automate registry generation in CI
564-
- Add registry as release asset for repos with releases
503+
## Status and Next Steps
504+
505+
The dynamic registry system is fully implemented. The package ships only a minimal bootstrap file that tells the system where to find remote model repositories. On first import, `modflow-devtools` attempts to auto-sync the default registries.
506+
507+
The next step is upstream integration: model repositories should automate registry generation in CI workflows.
565508

566509
## Cross-API Consistency
567510

@@ -764,6 +707,22 @@ $ mf models list --ref registry
764707
$ mf models list --source mf6/test --ref registry --verbose
765708
```
766709

710+
#### Clear Cached Registries
711+
712+
```bash
713+
# Clear all cached registries (with confirmation)
714+
$ mf models clear
715+
716+
# Clear specific source
717+
$ mf models clear --source mf6/test
718+
719+
# Clear specific source and ref
720+
$ mf models clear --source mf6/test --ref develop
721+
722+
# Skip confirmation prompt
723+
$ mf models clear --force
724+
```
725+
767726
### Registry Creation Tool
768727

769728
The `make_registry` tool uses a mode-based interface with **remote-first operation** by default:
@@ -772,7 +731,7 @@ The `make_registry` tool uses a mode-based interface with **remote-first operati
772731
```bash
773732
# Downloads repo and indexes subdirectory
774733
python -m modflow_devtools.models.make_registry \
775-
--mode version \
734+
\
776735
--repo MODFLOW-ORG/modflow6-testmodels \
777736
--ref master \
778737
--name mf6/test \
@@ -784,7 +743,7 @@ python -m modflow_devtools.models.make_registry \
784743
```bash
785744
# Downloads repo and indexes subdirectory
786745
python -m modflow_devtools.models.make_registry \
787-
--mode release \
746+
--asset-file mf6examples.zip \
788747
--repo MODFLOW-ORG/modflow6-examples \
789748
--ref current \
790749
--asset-file mf6examples.zip \
@@ -797,7 +756,7 @@ python -m modflow_devtools.models.make_registry \
797756
```bash
798757
# Downloads repo and indexes from root
799758
python -m modflow_devtools.models.make_registry \
800-
--mode version \
759+
\
801760
--repo MODFLOW-ORG/modflow6-testmodels \
802761
--ref master \
803762
--name mf6/test \
@@ -808,7 +767,7 @@ python -m modflow_devtools.models.make_registry \
808767
```bash
809768
# Uses existing local checkout
810769
python -m modflow_devtools.models.make_registry \
811-
--mode version \
770+
\
812771
--repo MODFLOW-ORG/modflow6-testmodels \
813772
--ref master \
814773
--name mf6/test \
@@ -822,7 +781,7 @@ python -m modflow_devtools.models.make_registry \
822781
- Relative path like `mf6` → downloads and uses as subdirectory
823782
- Existing local directory → uses local checkout (for testing)
824783
- Omitted → downloads and indexes repo root
825-
- **Mode-based interface**: Choose `--mode version` or `--mode release`
784+
- **Automatic mode detection**: Presence of `--asset-file` indicates release asset mode, otherwise version-controlled mode
826785
- **Automatic URL construction**: No manual URL typing required
827786
- **No git dependency**: Uses GitHub's zipball API
828787
- **Clear naming**: `--name` matches bootstrap file's `name` field
@@ -853,7 +812,7 @@ This allows testing against forks without modifying the bundled config!
853812
- name: Generate registry
854813
run: |
855814
python -m modflow_devtools.models.make_registry \
856-
--mode version \
815+
\
857816
--repo MODFLOW-ORG/modflow6-testmodels \
858817
--ref ${{ github.ref_name }} \
859818
--name mf6/test \
@@ -872,7 +831,7 @@ This allows testing against forks without modifying the bundled config!
872831
- name: Generate registry
873832
run: |
874833
python -m modflow_devtools.models.make_registry \
875-
--mode release \
834+
--asset-file mf6examples.zip \
876835
--repo MODFLOW-ORG/modflow6-examples \
877836
--ref ${{ github.ref_name }} \
878837
--asset-file mf6examples.zip \

0 commit comments

Comments
 (0)