From 33e875d065429b56b3fffb2435629707b167f0f2 Mon Sep 17 00:00:00 2001 From: arthurmccray Date: Mon, 14 Sep 2026 11:32:40 -0700 Subject: [PATCH 01/18] Opus code-review bugfixes --- emdatabase/catalogue.py | 6 ++- emdatabase/config.py | 77 +++++++++++++++++++++++++--- emdatabase/data/__init__.py | 27 +++++++++- emdatabase/downloadable_dataset.py | 2 +- emdatabase/metadata.py | 12 ++++- emdatabase/new_dataset.py | 9 +++- emdatabase/query.py | 8 ++- emdatabase/tests/test_config.py | 50 ++++++++++++++++++ emdatabase/tests/test_data_index.py | 59 +++++++++++++++++++++ emdatabase/tests/test_load_data.py | 23 +++++++++ emdatabase/tests/test_metadata.py | 11 ++++ emdatabase/tests/test_new_dataset.py | 5 +- emdatabase/tests/test_query.py | 7 +++ emdatabase/tests/test_widget.py | 22 ++++++++ emdatabase/widget.py | 4 +- 15 files changed, 301 insertions(+), 21 deletions(-) create mode 100644 emdatabase/tests/test_data_index.py diff --git a/emdatabase/catalogue.py b/emdatabase/catalogue.py index 8a59db5..868db6c 100644 --- a/emdatabase/catalogue.py +++ b/emdatabase/catalogue.py @@ -57,7 +57,11 @@ def resolve(name: str) -> DownloadableDataset | None: import emdatabase.data as data obj = getattr(data, str(name), None) - if not inspect.isclass(obj) or not issubclass(obj, DownloadableDataset): + if ( + not inspect.isclass(obj) + or obj is DownloadableDataset + or not issubclass(obj, DownloadableDataset) + ): return None return obj() diff --git a/emdatabase/config.py b/emdatabase/config.py index 86ac65e..7a47dfa 100644 --- a/emdatabase/config.py +++ b/emdatabase/config.py @@ -163,9 +163,12 @@ def _assign( self._record.append(("insert", path, None)) d[key] = value else: - if key not in d: + if key not in d or not isinstance(d[key], dict): if record: - self._record.append(("insert", path, None)) + if key in d: + self._record.append(("replace", path, d[key])) + else: + self._record.append(("insert", path, None)) d[key] = {} record = False self._assign(keys[1:], value, d[key], path, record=record) @@ -523,20 +526,65 @@ def check_key_val(key: str, val: Any, deprecations: dict = deprecations) -> tupl return key, val +def _without_env(document: Mapping, env: Mapping) -> dict: + """``document`` minus the entries the environment is currently supplying.""" + result: dict = {} + for key, value in document.items(): + if key in env: + from_env = env[key] + if isinstance(value, Mapping) and isinstance(from_env, Mapping): + nested = _without_env(value, from_env) + if nested: + result[key] = nested + continue + if value == from_env: + continue + result[key] = value + return result + + +def _dump(document: Mapping, path: Path | str | None = None) -> Path: + """Write ``document`` to a yaml file, creating the config directory.""" + path = Path(path) if path is not None else _config_dir() / "config.yaml" + path.parent.mkdir(parents=True, exist_ok=True) + with open(path, "w") as f: + yaml.dump(dict(document), f) + return path + + def write(path: Path | str | None = None) -> None: """Write the current configuration to a yaml file. + What the environment is supplying is left out: ``EMDATABASE_*`` holds for + the session that set it, and writing it here would keep it long after it is + unset. + Parameters ---------- path : Path or str, optional Path to write the yaml file to. Defaults to ``config.yaml`` in the config directory. """ - path = Path(path) if path is not None else _config_dir() / "config.yaml" - path.parent.mkdir(parents=True, exist_ok=True) + _dump(_without_env(config, collect_env()), path) - with open(path, "w") as f: - yaml.dump(config, f) + +def _persist_locations(updates: Mapping[str, str | None], remove: str | None = None) -> None: + """Merge these ``locations`` entries into the config file, and only these. + + Writing the whole live mapping would bake whatever the environment or an + open :class:`set` block is supplying into the file alongside the change the + caller asked for. + """ + path = _config_dir() / "config.yaml" + document = (_load_config_file(path) if path.exists() else None) or {} + configured = document.get("locations") + if not isinstance(configured, dict): + configured = {} + if remove is not None: + configured.pop(remove, None) + configured.update(updates) + document["locations"] = configured + _dump(document, path) # --------------------------------------------------------------------------- @@ -575,6 +623,13 @@ def _configured() -> dict[str, Path | None]: yaml could leave any other entry empty the same way. """ configured = get("locations", None) or {} + if not isinstance(configured, Mapping): + raise TypeError( + f"The locations config must be a mapping of name to directory, not " + f"{configured!r}. Set one entry at a time: " + f"{ENV_PREFIX}LOCATIONS__PERSONAL=/scratch, or " + 'config.set({"locations.personal": "/scratch"}).' + ) return { str(name): (Path(str(path)).expanduser() if path else None) for name, path in configured.items() @@ -712,7 +767,7 @@ def add_location(path: Path | str, name: str | None = None, persist: bool = True set({"locations": updated}) if persist: - write() + _persist_locations({name: str(expanded)}) return expanded @@ -766,7 +821,13 @@ def remove_location(name_or_path: Path | str, persist: bool = True) -> None: updated.setdefault("personal", None) set({"locations": updated}) if persist: - write() + _persist_locations({"personal": None} if name == "personal" else {}, remove=name) + from_env = collect_env().get("locations") + if isinstance(from_env, Mapping) and name in from_env: + warnings.warn( + f"{ENV_PREFIX}LOCATIONS__{name.upper()} still sets {name!r} in the " + "environment, so it comes back in a new session until that is unset." + ) def first_run_notice(directory: Path | None = None) -> None: diff --git a/emdatabase/data/__init__.py b/emdatabase/data/__init__.py index a932895..8dbc6ab 100644 --- a/emdatabase/data/__init__.py +++ b/emdatabase/data/__init__.py @@ -1,5 +1,7 @@ """Auto-generated dataset classes from YAML Files for downloading data.""" +import warnings + import yaml from emdatabase._create_stubs import build_docstring @@ -7,20 +9,41 @@ from emdatabase.metadata import DatasetMetadata, dataset_files __all__ = [] +_declared_in = {} for dataset_path in dataset_files(): data_dict_yaml = yaml.safe_load(dataset_path.read_text(encoding="utf-8")) + if not data_dict_yaml: + # An empty or comment-only index file is a problem for validation to + # report, not a reason for `import emdatabase` to fail. + warnings.warn(f"no dataset entries in {dataset_path}", stacklevel=2) + continue for name in data_dict_yaml: class_name = name.replace(" ", "_").replace("-", "_") - data_dict = data_dict_yaml[name] + data_dict = data_dict_yaml[name] or {} + if class_name in _declared_in: + warnings.warn( + f"skipping {name!r} in {dataset_path}: already declared in " + f"{_declared_in[class_name]}", + stacklevel=2, + ) + continue + try: + _metadata = DatasetMetadata.from_spec(data_dict, dataset_path) + except TypeError as error: + # A malformed entry is for validation to report; it should not make + # the package unimportable for everyone else. + warnings.warn(f"skipping {name!r} in {dataset_path}: {error}", stacklevel=2) + continue _new_class = type( class_name, (DownloadableDataset,), { "_spec": data_dict, "_origin": dataset_path, - "_metadata": DatasetMetadata.from_spec(data_dict, dataset_path), + "_metadata": _metadata, "__doc__": build_docstring(data_dict), }, ) globals()[class_name] = _new_class + _declared_in[class_name] = dataset_path __all__.append(class_name) diff --git a/emdatabase/downloadable_dataset.py b/emdatabase/downloadable_dataset.py index b17cc6e..67e9d32 100644 --- a/emdatabase/downloadable_dataset.py +++ b/emdatabase/downloadable_dataset.py @@ -576,7 +576,7 @@ def download( # In Jupyter (with the widget installed) a background download pops a # cancelable toast; the toast's monitor replaces the plain progress bar. monitor = finish = None - if progressbar: + if progressbar is True: # a caller's own Progress is theirs to drive try: from emdatabase.widget import _attach_toast diff --git a/emdatabase/metadata.py b/emdatabase/metadata.py index caca5d5..94cf9dd 100644 --- a/emdatabase/metadata.py +++ b/emdatabase/metadata.py @@ -174,6 +174,14 @@ def validate_document( "validating a dataset YAML needs jsonschema: pip install emdatabase[dev]" ) from error + if not isinstance(document, Mapping): + got = "nothing" if document is None else f"a {type(document).__name__}" + return [ + f"{_where(origin)}: document: expected a mapping of entry name to entry, got {got}" + ] + if not document: + return [f"{_where(origin)}: document: no entries"] + schema = load_schema() validator = validator_for(schema)(schema) problems = [ @@ -256,7 +264,9 @@ def format_size(size_bytes: int | None) -> str: value = float(size_bytes) unit = _SIZE_UNITS[0] for unit in _SIZE_UNITS: - if value < 1000 or unit == _SIZE_UNITS[-1]: + # The value as it will be shown, so 999_999 is "1.00 MB", not "1000.0 kB" + shown = value if unit == "B" else round(value, 1) + if shown < 1000 or unit == _SIZE_UNITS[-1]: break value /= 1000 if unit == "B": diff --git a/emdatabase/new_dataset.py b/emdatabase/new_dataset.py index a4f14d8..d1f44cc 100644 --- a/emdatabase/new_dataset.py +++ b/emdatabase/new_dataset.py @@ -32,6 +32,7 @@ import email.message import hashlib import re +import shutil import sys import tempfile import urllib.parse @@ -461,10 +462,14 @@ def main(argv: list[str] | None = None) -> int: size_bytes = content_length(url) checksum = args.checksum if checksum is None: - temporary = Path(tempfile.gettempdir()) / (filename or "download") + # A private directory: a predictable name in the shared temp dir would + # truncate whatever is already there, and two runs would collide. + directory = Path(tempfile.mkdtemp(prefix="emdatabase-")) + temporary = directory / (filename or "download") try: digest, downloaded, served, _ = download_md5(url, temporary) except OSError as error: + shutil.rmtree(directory, ignore_errors=True) print(f"could not download {url}: {error}") return 1 checksum = f"md5:{digest}" @@ -475,7 +480,7 @@ def main(argv: list[str] | None = None) -> int: if args.keep: print(f"kept {temporary}") else: - temporary.unlink(missing_ok=True) + shutil.rmtree(directory, ignore_errors=True) if not filename: filename = _ask("file name the download should be saved as", assume_yes=args.yes) diff --git a/emdatabase/query.py b/emdatabase/query.py index 6249fba..c5129ab 100644 --- a/emdatabase/query.py +++ b/emdatabase/query.py @@ -18,7 +18,7 @@ from __future__ import annotations -from collections.abc import Mapping +from collections.abc import Iterable, Mapping from typing import Any from emdatabase import catalogue @@ -146,7 +146,11 @@ def _value(ds: DownloadableDataset, row: dict, field: str) -> Any: def _matches(value: Any, wanted: Any) -> bool: if wanted is None or isinstance(wanted, bool): return value == wanted - wanted_values = [wanted] if isinstance(wanted, str) else list(wanted) + if isinstance(wanted, str) or not isinstance(wanted, Iterable): + # A scalar criterion: filter(voltage=200) asks the same as ["200"]. + wanted_values = [wanted] + else: + wanted_values = list(wanted) if isinstance(value, (tuple, list, Mapping)): # technique, tags and authors: a dataset matches if it carries any one # of them. diff --git a/emdatabase/tests/test_config.py b/emdatabase/tests/test_config.py index 9c04130..a2e1c67 100644 --- a/emdatabase/tests/test_config.py +++ b/emdatabase/tests/test_config.py @@ -309,3 +309,53 @@ def test_the_top_level_re_exports_are_the_config_functions(): assert emdatabase.add_location is config.add_location assert emdatabase.remove_location is config.remove_location assert emdatabase.locations is config.locations + + +def test_set_replaces_a_nested_key_whose_parent_is_empty(tmp_path): + # A bare "locations:" in the yaml parses to None, which a nested set has to + # stand in a dict for rather than trip over. + _write_yaml(tmp_path, locations=None) + config.refresh() + with config.set({"locations.personal": str(tmp_path / "scratch")}): + assert config.data_dir() == tmp_path / "scratch" + assert config.get("locations") is None # rolled back to what the file said + + +def test_locations_that_is_not_a_mapping_says_so(monkeypatch): + monkeypatch.setenv("EMDATABASE_LOCATIONS", "/one/directory") + config.refresh() + with pytest.raises(TypeError, match="must be a mapping"): + config.locations() + + +def test_write_leaves_out_what_the_environment_is_supplying(tmp_path, monkeypatch): + monkeypatch.setenv("EMDATABASE_LOCATIONS__PERSONAL", str(tmp_path / "from-env")) + config.refresh() + config.write() + written = yaml.safe_load((tmp_path / "config" / "config.yaml").read_text()) + assert "personal" not in written.get("locations", {}) + + +def test_add_location_persists_only_its_own_change(tmp_path, monkeypatch): + monkeypatch.setenv("EMDATABASE_LOCATIONS__PERSONAL", str(tmp_path / "from-env")) + config.refresh() + config.add_location(_dir(tmp_path, "example_data")) + written = yaml.safe_load((tmp_path / "config" / "config.yaml").read_text()) + assert written["locations"] == {"example_data": str(tmp_path / "example_data")} + + +def test_add_location_keeps_the_entries_already_in_the_file(tmp_path): + config.add_location(_dir(tmp_path, "first")) + config.add_location(_dir(tmp_path, "second")) + written = yaml.safe_load((tmp_path / "config" / "config.yaml").read_text()) + assert written["locations"] == { + "first": str(tmp_path / "first"), + "second": str(tmp_path / "second"), + } + + +def test_remove_location_says_the_environment_still_sets_it(tmp_path, monkeypatch): + monkeypatch.setenv("EMDATABASE_LOCATIONS__GROUP", str(_dir(tmp_path, "group"))) + config.refresh() + with pytest.warns(UserWarning, match="still sets 'group'"): + config.remove_location("group") diff --git a/emdatabase/tests/test_data_index.py b/emdatabase/tests/test_data_index.py new file mode 100644 index 0000000..a586bf0 --- /dev/null +++ b/emdatabase/tests/test_data_index.py @@ -0,0 +1,59 @@ +"""How ``emdatabase.data`` handles a bad index file. + +The classes are built at import, so anything that raises here takes down +``import emdatabase`` for every user. A file that is empty, an entry with no +body and a name declared twice are all problems for validation to report, and +none of them is a reason for the package not to import. +""" + +import importlib +import textwrap + +import pytest + +ENTRY = textwrap.dedent( + """\ + {name}: + description: A 4D-STEM dataset of something. + source: https://zenodo.org/records/0000000/files + file: {name}.zspy + """ +) + + +@pytest.fixture +def index(tmp_path, monkeypatch): + """Rebuild ``emdatabase.data`` from index files given as ``stem=text``.""" + import emdatabase.metadata as metadata + + def build(**files): + paths = [] + for stem, text in files.items(): + path = tmp_path / f"{stem}.yaml" + path.write_text(text, encoding="utf-8") + paths.append(path) + monkeypatch.setattr(metadata, "dataset_files", lambda: paths) + return importlib.reload(importlib.import_module("emdatabase.data")) + + yield build + monkeypatch.undo() + importlib.reload(importlib.import_module("emdatabase.data")) + + +def test_an_empty_index_file_warns_and_the_rest_still_loads(index): + with pytest.warns(UserWarning, match="no dataset entries"): + data = index(a_empty="# nothing here yet\n", b_real=ENTRY.format(name="Real")) + assert data.__all__ == ["Real"] + + +def test_an_entry_with_no_body_is_skipped(index): + with pytest.warns(UserWarning, match="skipping 'Hollow'"): + data = index(entries="Hollow:\n" + ENTRY.format(name="Real")) + assert data.__all__ == ["Real"] + + +def test_a_name_declared_twice_is_counted_once(index): + with pytest.warns(UserWarning, match="already declared"): + data = index(a_first=ENTRY.format(name="Same"), b_second=ENTRY.format(name="Same")) + assert data.__all__ == ["Same"] + assert data.Same._origin.name == "a_first.yaml" # the first file wins diff --git a/emdatabase/tests/test_load_data.py b/emdatabase/tests/test_load_data.py index ac4efbc..b8895b7 100644 --- a/emdatabase/tests/test_load_data.py +++ b/emdatabase/tests/test_load_data.py @@ -401,3 +401,26 @@ def test_a_malformed_entry_warns_instead_of_vanishing(monkeypatch): with pytest.warns(UserWarning, match="BrokenDataset"): found = dict(catalogue.datasets()) assert "BrokenDataset" not in found # skipped, but not silently + + +def test_a_caller_s_progress_bar_is_not_replaced_by_the_toast(tmp_path, monkeypatch): + """The Jupyter toast stands in for the default bar, not for one passed in.""" + import emdatabase.widget as widget_mod + + dataset = getattr(data, TINY_DATASET)() + seen = [] + + def record(destination=None, progressbar=True, chunk_size=4096, version=None, refresh=False): + seen.append(progressbar) + target = tmp_path / dataset.file + target.write_bytes(b"payload") + return str(target) + + monitor = object() + monkeypatch.setattr(dataset, "_retrieve", record) + monkeypatch.setattr(widget_mod, "_attach_toast", lambda label: (monitor, None)) + + mine = object() + dataset.download(destination=tmp_path, progressbar=mine).wait(2) + dataset.download(destination=tmp_path, progressbar=True, refresh=True).wait(2) + assert seen == [mine, monitor] diff --git a/emdatabase/tests/test_metadata.py b/emdatabase/tests/test_metadata.py index 2e1ceb8..558a8fd 100644 --- a/emdatabase/tests/test_metadata.py +++ b/emdatabase/tests/test_metadata.py @@ -178,6 +178,8 @@ def test_tags_and_authors_are_converted(): (104291721, "104.3 MB"), (1104287335, "1.10 GB"), (5748299565, "5.75 GB"), + (999999, "1.00 MB"), # rounds up into the next unit, not "1000.0 kB" + (999999999, "1.00 GB"), ], ) def test_format_size(size_bytes, expected): @@ -361,3 +363,12 @@ def test_str_without_a_description(): text = str(DatasetMetadata(description="", source="https://example.com", file="d.zspy")) assert text.startswith("d.zspy\n\n") assert "source: https://example.com" in text + + +def test_validate_document_reports_an_empty_document(tmp_path): + path = tmp_path / "Empty.yaml" + path.write_text("# nothing here yet\n", encoding="utf-8") + problems = validate_file(path) + assert len(problems) == 1 + assert "Empty.yaml" in problems[0] + assert validate_document({}) == ["dataset entry: document: no entries"] diff --git a/emdatabase/tests/test_new_dataset.py b/emdatabase/tests/test_new_dataset.py index 10efc00..14c09c4 100644 --- a/emdatabase/tests/test_new_dataset.py +++ b/emdatabase/tests/test_new_dataset.py @@ -10,7 +10,6 @@ """ import hashlib -from pathlib import Path import pytest import yaml @@ -551,7 +550,9 @@ def test_keep_leaves_the_temporary_download(server, tmp_path, monkeypatch): "--keep", ] ) - assert Path(tmp_path / "scratch" / "MyData.zspy").read_bytes() == CONTENT + kept = list((tmp_path / "scratch").glob("emdatabase-*/MyData.zspy")) + assert len(kept) == 1 + assert kept[0].read_bytes() == CONTENT def test_write_document_matches_the_hand_written_style(tmp_path): diff --git a/emdatabase/tests/test_query.py b/emdatabase/tests/test_query.py index ad65348..261dd89 100644 --- a/emdatabase/tests/test_query.py +++ b/emdatabase/tests/test_query.py @@ -192,3 +192,10 @@ def test_public_names_survive_a_submodule_walk(): assert not isinstance(exported, types.ModuleType) or name in ("data", "config"), ( f"emdatabase.{name} was replaced by a submodule of the same name" ) + + +def test_filter_accepts_a_scalar_that_is_not_a_string(): + voltage = set(names(emdatabase.filter(voltage="200 kV"))) + assert voltage + assert set(names(emdatabase.filter(voltage=["200 kV"]))) == voltage + assert emdatabase.filter(voltage=200) == [] # a bare int matches nothing, but does not raise diff --git a/emdatabase/tests/test_widget.py b/emdatabase/tests/test_widget.py index 87b65e2..9c3f333 100644 --- a/emdatabase/tests/test_widget.py +++ b/emdatabase/tests/test_widget.py @@ -354,3 +354,25 @@ def test_widget_download_end_to_end(tmp_path): ds = catalogue.resolve(TINY_DATASET) assert ds is not None assert (tmp_path / ds.file).exists() + + +def test_resolve_does_not_build_the_base_class(): + """DownloadableDataset is in the emdatabase.data namespace, but is not an entry.""" + assert catalogue.resolve("DownloadableDataset") is None + assert catalogue.resolve("NotADataset") is None + + +def test_a_card_delete_that_fails_warns_rather_than_claiming_success(monkeypatch): + pytest.importorskip("anywidget") + ds = catalogue.resolve(TINY_DATASET) + assert ds is not None + + def boom(version=None): + raise PermissionError("read-only location") + + import emdatabase.widget as widget_mod + + monkeypatch.setattr(ds, "delete", boom) + widget = widget_mod.card(ds) + with pytest.warns(UserWarning, match="could not delete"): + widget._on_command({"new": {"action": "delete", "nonce": 1}}) diff --git a/emdatabase/widget.py b/emdatabase/widget.py index 41bb1a7..ce934b4 100644 --- a/emdatabase/widget.py +++ b/emdatabase/widget.py @@ -330,8 +330,8 @@ def _on_command(self, change): elif action == "delete": try: self._dataset.delete(version=version) - except Exception: - pass + except OSError as error: # read-only dir, permissions, a vanished file + warnings.warn(f"could not delete {self._name}: {error}", stacklevel=2) self.refresh() elif action == "refresh": self.refresh() From b99391f6c8bc6660e933ed454275d50ad80d45f9 Mon Sep 17 00:00:00 2001 From: arthurmccray Date: Mon, 14 Sep 2026 13:14:35 -0700 Subject: [PATCH 02/18] index guards --- emdatabase/__init__.py | 6 ++- emdatabase/_create_stubs.py | 34 ++++++++--------- emdatabase/config.py | 29 +------------- emdatabase/data/__init__.py | 57 +++++++--------------------- emdatabase/index/json-schema.json | 3 +- emdatabase/metadata.py | 40 ++++++++++++++++--- emdatabase/tests/test_config.py | 15 -------- emdatabase/tests/test_data_index.py | 59 ----------------------------- emdatabase/tests/test_metadata.py | 38 ++++++++++++++++++- emdatabase/tests/test_weights.py | 3 +- emdatabase/tests/test_widget.py | 20 ++++++++++ 11 files changed, 129 insertions(+), 175 deletions(-) delete mode 100644 emdatabase/tests/test_data_index.py diff --git a/emdatabase/__init__.py b/emdatabase/__init__.py index c41a1f6..05d47d7 100644 --- a/emdatabase/__init__.py +++ b/emdatabase/__init__.py @@ -50,7 +50,7 @@ class _EmDatabaseModule(_ModuleType): def _repr_mimebundle_(self, include=None, exclude=None, **kwargs): try: widget = browse() - except Exception: + except ImportError: return { "text/plain": ( "emdatabase — install the interactive browser with " @@ -58,6 +58,10 @@ def _repr_mimebundle_(self, include=None, exclude=None, **kwargs): "emdatabase.browse()." ) } + except Exception as error: + # Anything else is a real problem - a misconfigured locations key, + # say - and its message is the only thing that explains the repr. + return {"text/plain": f"emdatabase — the browser could not be built: {error}"} return widget._repr_mimebundle_(**kwargs) diff --git a/emdatabase/_create_stubs.py b/emdatabase/_create_stubs.py index 529b738..804600b 100644 --- a/emdatabase/_create_stubs.py +++ b/emdatabase/_create_stubs.py @@ -9,9 +9,7 @@ import sys from pathlib import Path -import yaml - -from emdatabase.metadata import dataset_files +from emdatabase.metadata import index_entries STUB_PATH = Path(__file__).parent / "data" / "__init__.pyi" @@ -47,24 +45,22 @@ def build_pyi_stub() -> str: dataset_classes = [] - for dataset_path in dataset_files(): - data_dict_yaml = yaml.safe_load(dataset_path.read_text(encoding="utf-8")) - for name in data_dict_yaml: - data_dict = data_dict_yaml[name] - class_name = name.replace(" ", "_").replace("-", "_") - description = build_docstring(data_dict) - - stub_lines.append(f"class {class_name}(DownloadableDataset):") - stub_lines.append(' """') - stub_lines.append(f" {name}") - if description: - stub_lines.append("") - stub_lines.append(f" {description}") - stub_lines.append(' """') - stub_lines.append(" ...") + # The same entries emdatabase.data builds classes from, so the stub cannot + # claim a class the loader skipped or miss one it made. + for entry in index_entries(): + description = build_docstring(entry.spec) + + stub_lines.append(f"class {entry.class_name}(DownloadableDataset):") + stub_lines.append(' """') + stub_lines.append(f" {entry.name}") + if description: stub_lines.append("") + stub_lines.append(f" {description}") + stub_lines.append(' """') + stub_lines.append(" ...") + stub_lines.append("") - dataset_classes.append(class_name) + dataset_classes.append(entry.class_name) stub_lines.append(f"__all__ = {dataset_classes}") return "\n".join(stub_lines) diff --git a/emdatabase/config.py b/emdatabase/config.py index 7a47dfa..45b051e 100644 --- a/emdatabase/config.py +++ b/emdatabase/config.py @@ -526,23 +526,6 @@ def check_key_val(key: str, val: Any, deprecations: dict = deprecations) -> tupl return key, val -def _without_env(document: Mapping, env: Mapping) -> dict: - """``document`` minus the entries the environment is currently supplying.""" - result: dict = {} - for key, value in document.items(): - if key in env: - from_env = env[key] - if isinstance(value, Mapping) and isinstance(from_env, Mapping): - nested = _without_env(value, from_env) - if nested: - result[key] = nested - continue - if value == from_env: - continue - result[key] = value - return result - - def _dump(document: Mapping, path: Path | str | None = None) -> Path: """Write ``document`` to a yaml file, creating the config directory.""" path = Path(path) if path is not None else _config_dir() / "config.yaml" @@ -555,17 +538,13 @@ def _dump(document: Mapping, path: Path | str | None = None) -> Path: def write(path: Path | str | None = None) -> None: """Write the current configuration to a yaml file. - What the environment is supplying is left out: ``EMDATABASE_*`` holds for - the session that set it, and writing it here would keep it long after it is - unset. - Parameters ---------- path : Path or str, optional Path to write the yaml file to. Defaults to ``config.yaml`` in the config directory. """ - _dump(_without_env(config, collect_env()), path) + _dump(config, path) def _persist_locations(updates: Mapping[str, str | None], remove: str | None = None) -> None: @@ -822,12 +801,6 @@ def remove_location(name_or_path: Path | str, persist: bool = True) -> None: set({"locations": updated}) if persist: _persist_locations({"personal": None} if name == "personal" else {}, remove=name) - from_env = collect_env().get("locations") - if isinstance(from_env, Mapping) and name in from_env: - warnings.warn( - f"{ENV_PREFIX}LOCATIONS__{name.upper()} still sets {name!r} in the " - "environment, so it comes back in a new session until that is unset." - ) def first_run_notice(directory: Path | None = None) -> None: diff --git a/emdatabase/data/__init__.py b/emdatabase/data/__init__.py index 8dbc6ab..864f62e 100644 --- a/emdatabase/data/__init__.py +++ b/emdatabase/data/__init__.py @@ -1,49 +1,20 @@ """Auto-generated dataset classes from YAML Files for downloading data.""" -import warnings - -import yaml - from emdatabase._create_stubs import build_docstring from emdatabase.downloadable_dataset import DownloadableDataset -from emdatabase.metadata import DatasetMetadata, dataset_files +from emdatabase.metadata import index_entries __all__ = [] -_declared_in = {} -for dataset_path in dataset_files(): - data_dict_yaml = yaml.safe_load(dataset_path.read_text(encoding="utf-8")) - if not data_dict_yaml: - # An empty or comment-only index file is a problem for validation to - # report, not a reason for `import emdatabase` to fail. - warnings.warn(f"no dataset entries in {dataset_path}", stacklevel=2) - continue - for name in data_dict_yaml: - class_name = name.replace(" ", "_").replace("-", "_") - data_dict = data_dict_yaml[name] or {} - if class_name in _declared_in: - warnings.warn( - f"skipping {name!r} in {dataset_path}: already declared in " - f"{_declared_in[class_name]}", - stacklevel=2, - ) - continue - try: - _metadata = DatasetMetadata.from_spec(data_dict, dataset_path) - except TypeError as error: - # A malformed entry is for validation to report; it should not make - # the package unimportable for everyone else. - warnings.warn(f"skipping {name!r} in {dataset_path}: {error}", stacklevel=2) - continue - _new_class = type( - class_name, - (DownloadableDataset,), - { - "_spec": data_dict, - "_origin": dataset_path, - "_metadata": _metadata, - "__doc__": build_docstring(data_dict), - }, - ) - globals()[class_name] = _new_class - _declared_in[class_name] = dataset_path - __all__.append(class_name) +for _entry in index_entries(): + _new_class = type( + _entry.class_name, + (DownloadableDataset,), + { + "_spec": _entry.spec, + "_origin": _entry.origin, + "_metadata": _entry.metadata, + "__doc__": build_docstring(_entry.spec), + }, + ) + globals()[_entry.class_name] = _new_class + __all__.append(_entry.class_name) diff --git a/emdatabase/index/json-schema.json b/emdatabase/index/json-schema.json index 27a6e51..1d76293 100644 --- a/emdatabase/index/json-schema.json +++ b/emdatabase/index/json-schema.json @@ -3,8 +3,9 @@ "$id": "https://example.com/schemas/em-dataset.collection.schema.json", "title": "EM Dataset Collection", "type": "object", + "additionalProperties": false, "patternProperties": { - "^.+$": { + "^[A-Za-z_][A-Za-z0-9_ -]*$": { "type": "object", "properties": { "description": { diff --git a/emdatabase/metadata.py b/emdatabase/metadata.py index 94cf9dd..80da5ed 100644 --- a/emdatabase/metadata.py +++ b/emdatabase/metadata.py @@ -12,10 +12,10 @@ called on disk. This module also owns the small amount of shared knowledge about where the -dataset files live - :func:`dataset_files`, :func:`load_schema`, -:func:`load_vendors`, :func:`techniques` - so the loader, the stub generator, -the docs form and the tests all read the same directory and the same technique -vocabulary the same way, and the one check a candidate file has to pass - +dataset files live - :func:`dataset_files`, :func:`index_entries`, +:func:`load_schema`, :func:`load_vendors`, :func:`techniques` - so the loader, +the stub generator, the docs form and the tests all read the same directory and +the same technique vocabulary the same way, and the one check a candidate file has to pass - :func:`validate_document`, :func:`validate_file` - so the test suite, the issue-form workflow and ``emdatabase.new_dataset`` accept and reject exactly the same files. @@ -26,10 +26,10 @@ import difflib import textwrap import warnings -from collections.abc import Iterable, Mapping +from collections.abc import Iterable, Iterator, Mapping from dataclasses import dataclass, field, fields from pathlib import Path -from typing import Any +from typing import Any, NamedTuple import yaml @@ -53,6 +53,34 @@ def dataset_files() -> list[Path]: return sorted(p for p in INDEX_DIR.rglob("*.y*ml") if p.name not in NON_DATASET_FILES) +class IndexEntry(NamedTuple): + """One usable entry from the index, as :func:`index_entries` yields it.""" + + name: str + class_name: str + spec: dict[str, Any] + origin: Path + metadata: DatasetMetadata + + +def index_entries() -> Iterator[IndexEntry]: + """Every entry in the index files, in file then declaration order. + + The one place that decides which entries exist, so the classes + :mod:`emdatabase.data` builds and the stub generated for them cannot + disagree about the set. The files ship with the package and CI validates + all of them, so a malformed one raises here rather than being skipped: that + is a broken release, not something a user can cause or fix. + """ + for path in dataset_files(): + document = yaml.safe_load(path.read_text(encoding="utf-8")) + for name, spec in document.items(): + class_name = str(name).replace(" ", "_").replace("-", "_") + yield IndexEntry( + str(name), class_name, spec, path, DatasetMetadata.from_spec(spec, path) + ) + + def load_schema() -> dict[str, Any]: """The JSON schema the dataset YAML is validated against.""" import json diff --git a/emdatabase/tests/test_config.py b/emdatabase/tests/test_config.py index a2e1c67..96e7757 100644 --- a/emdatabase/tests/test_config.py +++ b/emdatabase/tests/test_config.py @@ -328,14 +328,6 @@ def test_locations_that_is_not_a_mapping_says_so(monkeypatch): config.locations() -def test_write_leaves_out_what_the_environment_is_supplying(tmp_path, monkeypatch): - monkeypatch.setenv("EMDATABASE_LOCATIONS__PERSONAL", str(tmp_path / "from-env")) - config.refresh() - config.write() - written = yaml.safe_load((tmp_path / "config" / "config.yaml").read_text()) - assert "personal" not in written.get("locations", {}) - - def test_add_location_persists_only_its_own_change(tmp_path, monkeypatch): monkeypatch.setenv("EMDATABASE_LOCATIONS__PERSONAL", str(tmp_path / "from-env")) config.refresh() @@ -352,10 +344,3 @@ def test_add_location_keeps_the_entries_already_in_the_file(tmp_path): "first": str(tmp_path / "first"), "second": str(tmp_path / "second"), } - - -def test_remove_location_says_the_environment_still_sets_it(tmp_path, monkeypatch): - monkeypatch.setenv("EMDATABASE_LOCATIONS__GROUP", str(_dir(tmp_path, "group"))) - config.refresh() - with pytest.warns(UserWarning, match="still sets 'group'"): - config.remove_location("group") diff --git a/emdatabase/tests/test_data_index.py b/emdatabase/tests/test_data_index.py deleted file mode 100644 index a586bf0..0000000 --- a/emdatabase/tests/test_data_index.py +++ /dev/null @@ -1,59 +0,0 @@ -"""How ``emdatabase.data`` handles a bad index file. - -The classes are built at import, so anything that raises here takes down -``import emdatabase`` for every user. A file that is empty, an entry with no -body and a name declared twice are all problems for validation to report, and -none of them is a reason for the package not to import. -""" - -import importlib -import textwrap - -import pytest - -ENTRY = textwrap.dedent( - """\ - {name}: - description: A 4D-STEM dataset of something. - source: https://zenodo.org/records/0000000/files - file: {name}.zspy - """ -) - - -@pytest.fixture -def index(tmp_path, monkeypatch): - """Rebuild ``emdatabase.data`` from index files given as ``stem=text``.""" - import emdatabase.metadata as metadata - - def build(**files): - paths = [] - for stem, text in files.items(): - path = tmp_path / f"{stem}.yaml" - path.write_text(text, encoding="utf-8") - paths.append(path) - monkeypatch.setattr(metadata, "dataset_files", lambda: paths) - return importlib.reload(importlib.import_module("emdatabase.data")) - - yield build - monkeypatch.undo() - importlib.reload(importlib.import_module("emdatabase.data")) - - -def test_an_empty_index_file_warns_and_the_rest_still_loads(index): - with pytest.warns(UserWarning, match="no dataset entries"): - data = index(a_empty="# nothing here yet\n", b_real=ENTRY.format(name="Real")) - assert data.__all__ == ["Real"] - - -def test_an_entry_with_no_body_is_skipped(index): - with pytest.warns(UserWarning, match="skipping 'Hollow'"): - data = index(entries="Hollow:\n" + ENTRY.format(name="Real")) - assert data.__all__ == ["Real"] - - -def test_a_name_declared_twice_is_counted_once(index): - with pytest.warns(UserWarning, match="already declared"): - data = index(a_first=ENTRY.format(name="Same"), b_second=ENTRY.format(name="Same")) - assert data.__all__ == ["Same"] - assert data.Same._origin.name == "a_first.yaml" # the first file wins diff --git a/emdatabase/tests/test_metadata.py b/emdatabase/tests/test_metadata.py index 558a8fd..3620eef 100644 --- a/emdatabase/tests/test_metadata.py +++ b/emdatabase/tests/test_metadata.py @@ -34,7 +34,7 @@ DATASET_FILES = dataset_files() SCHEMA = load_schema() -ENTRY_SCHEMA = SCHEMA["patternProperties"]["^.+$"] +ENTRY_SCHEMA = next(iter(SCHEMA["patternProperties"].values())) VENDORS = load_vendors() TECHNIQUES = techniques() @@ -43,7 +43,7 @@ def entries(): """``(file, name, spec)`` for every entry in every dataset YAML.""" for path in DATASET_FILES: document = yaml.safe_load(path.read_text(encoding="utf-8")) - for name, spec in document.items(): + for name, spec in (document or {}).items(): yield path, name, spec @@ -60,6 +60,40 @@ def test_yaml_is_valid(path): assert validate_file(path) == [] +@pytest.mark.parametrize("path", DATASET_FILES, ids=lambda p: p.name) +def test_index_file_declares_entries(path): + """emdatabase.data reads these directly; an empty one is an import error.""" + document = yaml.safe_load(path.read_text(encoding="utf-8")) + assert isinstance(document, dict) and document, f"{path.name} declares no entries" + + +def test_entry_names_are_unique_across_files(): + """A name in two files would shadow, and be counted twice in the catalogue.""" + names = [name for _, name, _ in ENTRIES] + assert not [n for n in names if names.count(n) > 1] + + +def test_an_entry_name_no_class_can_have_is_rejected(): + """The names become classes in emdatabase.data, so the schema restricts them.""" + entry = {"description": "d", "source": "https://example.com", "file": "f.zspy"} + assert validate_document({"Fe3O4 (2021)": entry}) + assert validate_document({"2D-MoS2": entry}) + assert validate_document({"Fine_Name": entry}) == [] + + +def test_the_stub_lists_exactly_the_classes_the_loader_builds(): + """CI runs `_create_stubs --check`; both sides read metadata.index_entries.""" + import emdatabase.data as data + from emdatabase._create_stubs import build_pyi_stub + + declared = [ + line.removeprefix("class ").partition("(")[0] + for line in build_pyi_stub().splitlines() + if line.startswith("class ") + ] + assert declared == data.__all__ + + def test_template_is_valid(): """The template is not a dataset, so nothing else here looks at it; a placeholder that does not validate is a contributor's first impression.""" diff --git a/emdatabase/tests/test_weights.py b/emdatabase/tests/test_weights.py index bfc4587..40dfbe2 100644 --- a/emdatabase/tests/test_weights.py +++ b/emdatabase/tests/test_weights.py @@ -188,7 +188,8 @@ def test_an_unquoted_version_key_is_reported(): def test_the_model_schema_and_the_dataclass_agree(): """``class`` is a keyword, so the field is ``class_``; nothing else differs.""" - model_schema = load_schema()["patternProperties"]["^.+$"]["properties"]["model"] + entry_schema = next(iter(load_schema()["patternProperties"].values())) + model_schema = entry_schema["properties"]["model"] assert list(model_schema["properties"]) == ["class", "framework", "quantem"] assert model_schema["required"] == ["class", "framework"] diff --git a/emdatabase/tests/test_widget.py b/emdatabase/tests/test_widget.py index 9c3f333..bfdb967 100644 --- a/emdatabase/tests/test_widget.py +++ b/emdatabase/tests/test_widget.py @@ -376,3 +376,23 @@ def boom(version=None): widget = widget_mod.card(ds) with pytest.warns(UserWarning, match="could not delete"): widget._on_command({"new": {"action": "delete", "nonce": 1}}) + + +def test_module_display_reports_a_real_error_rather_than_install_advice(monkeypatch): + """A misconfigured locations key is not a missing anywidget.""" + + def _boom(): + raise TypeError("The locations config must be a mapping of name to directory") + + monkeypatch.setattr(emdatabase, "browse", _boom) + text = emdatabase._repr_mimebundle_()["text/plain"] + assert "must be a mapping" in text + assert "pip install" not in text + + +def test_module_display_still_advises_installing_the_widget(monkeypatch): + def _boom(): + raise ImportError("no anywidget") + + monkeypatch.setattr(emdatabase, "browse", _boom) + assert "pip install" in emdatabase._repr_mimebundle_()["text/plain"] From 86a03e539487d853a4adacda027c63defb275fdf Mon Sep 17 00:00:00 2001 From: arthurmccray Date: Mon, 14 Sep 2026 13:32:25 -0700 Subject: [PATCH 03/18] fixing CI --- .github/scripts/issue_to_yaml.py | 4 +++- .github/workflows/build.yml | 7 +------ .github/workflows/documentation.yaml | 1 + .github/workflows/publish.yml | 9 +++++---- emdatabase/tests/test_issue_route.py | 21 +++++++++++++++++++++ 5 files changed, 31 insertions(+), 11 deletions(-) diff --git a/.github/scripts/issue_to_yaml.py b/.github/scripts/issue_to_yaml.py index 603650c..f55aaab 100644 --- a/.github/scripts/issue_to_yaml.py +++ b/.github/scripts/issue_to_yaml.py @@ -139,7 +139,9 @@ def build_yaml(data): source, filename, link = split_url(url) if not source: sys.exit(f"{data['URL']!r} is not a link to a file") - filename = data["File Name"] or filename + # Whoever opened the issue typed this, and it is joined onto a directory + # later (check_latest_weights.py), so keep the name and nothing else. + filename = Path(data["File Name"] or filename).name if not filename: sys.exit(f"{data['URL']!r} does not end in a file name; fill in --File Name--") # The issue may already carry the size; without it the server is asked for diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 2d9a95c..74523f8 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -38,7 +38,7 @@ jobs: run: python -m emdatabase._create_stubs --check build-with-pip: - name: ${{ matrix.os }}-py${{ matrix.python-version }}${{ matrix.LABEL }} + name: ${{ matrix.os }}-py${{ matrix.python-version }} runs-on: ${{ matrix.os }} timeout-minutes: 15 strategy: @@ -71,11 +71,6 @@ jobs: run: | pip install -U -e .'[tests]' - - name: Install oldest supported versions - if: contains(matrix.LABEL, 'oldest') - run: | - pip install ${{ matrix.DEPENDENCIES }} - - name: Display Python, pip and package versions run: | python -V diff --git a/.github/workflows/documentation.yaml b/.github/workflows/documentation.yaml index 375e3da..c641a87 100644 --- a/.github/workflows/documentation.yaml +++ b/.github/workflows/documentation.yaml @@ -47,6 +47,7 @@ jobs: - name: Sphinx build timeout-minutes: 15 run: | + set -o pipefail make html -C docs 2>&1 | tee sphinx.log # A failed gallery example only warns (see conf.py), so say so where a diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index cd3dc2b..484e8cf 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -48,16 +48,17 @@ jobs: sys.exit("json-schema.json missing from the wheel") PY - - name: Check the version matches the release tag - if: github.event_name == 'release' + - name: Check the version is a release, and matches the tag run: | python - <<'PY' - import os, sys, tomllib + import os, re, sys, tomllib with open("pyproject.toml", "rb") as f: version = tomllib.load(f)["project"]["version"] tag = os.environ["TAG"].lstrip("v") print(f"pyproject version {version!r}, release tag {tag!r}") - if version != tag: + if not re.fullmatch(r"\d+\.\d+\.\d+", version): + sys.exit(f"{version} is not a release version, and PyPI never frees one up again") + if tag and version != tag: sys.exit(f"version mismatch: pyproject says {version}, tag says {tag}") PY env: diff --git a/emdatabase/tests/test_issue_route.py b/emdatabase/tests/test_issue_route.py index f21606c..9f7875e 100644 --- a/emdatabase/tests/test_issue_route.py +++ b/emdatabase/tests/test_issue_route.py @@ -200,6 +200,27 @@ def test_issue_takes_today_when_the_version_date_is_blank(parse): assert list(document[name]["versions"]) == [datetime.date.today().strftime("%y%m%d")] +def test_issue_file_name_keeps_only_the_name(parse): + """The File Name is typed by whoever opened the issue, and is later joined + onto a directory.""" + body = _issue_body( + **{ + "--Dataset Name--": "MgONanoCrystals", + "--Authors--": "Jane Doe; University of Somewhere", + "--URL--": "https://zenodo.org/records/0000000/files/MgONanoCrystals.zspy", + "--File Name--": "/etc/passwd", + "--Checksum--": "md5:df9376d5c020a23f0f7f51cfe79f303f", + "--Description--": "A 4D-STEM dataset of MgO nanocrystals.", + "--Detector Manufacturer--": "Direct Electron", + "Dataset License": "CC-BY-4.0", + "Technique": _ticked("4D-STEM"), + } + ) + document, name, problems = parse(body) + assert problems == [] + assert document[name]["file"] == "passwd" + + def test_issue_drive_link_becomes_url_plus_file_name(parse): body = _issue_body( **{ From 650e7ddb2527a84c8e901fff911f4d3aef5a75ae Mon Sep 17 00:00:00 2001 From: arthurmccray Date: Mon, 14 Sep 2026 14:25:39 -0700 Subject: [PATCH 04/18] simplifying config --- emdatabase/config.py | 369 +++++--------------------------- emdatabase/tests/test_config.py | 13 ++ 2 files changed, 64 insertions(+), 318 deletions(-) diff --git a/emdatabase/config.py b/emdatabase/config.py index 45b051e..2d4fea6 100644 --- a/emdatabase/config.py +++ b/emdatabase/config.py @@ -55,7 +55,7 @@ from collections.abc import Iterator, Mapping, Sequence from dataclasses import dataclass from pathlib import Path -from typing import Annotated, Any, Literal, Union +from typing import Annotated, Any import pooch import yaml @@ -67,8 +67,7 @@ PATH = Path(os.getenv("EMDATABASE_CONFIG", "~/.config/emdatabase")).expanduser() config: dict = {} -defaults: list[Mapping] = [] -deprecations: dict[str, str | None] = {} +defaults: dict = yaml.safe_load(Path(__file__).with_name("emdatabase.yaml").read_text()) def _config_dir() -> Path: @@ -81,39 +80,21 @@ def _config_dir() -> Path: class set: - """Temporarily set configuration values within a context manager + """Set configuration values, for the process or, as a context manager, for a block Parameters ---------- - arg : mapping or None, optional - A mapping of configuration key-value pairs to set. - **kwargs : - Additional key-value pairs to set. If ``arg`` is provided, values set - in ``arg`` will be applied before those in ``kwargs``. - Double-underscores (``__``) in keyword arguments will be replaced with - ``.``, allowing nested values to be easily set. + arg : mapping + Configuration key-value pairs to set. A dotted key such as + ``"locations.personal"`` sets a nested value. """ - def __init__( - self, - arg: Union[Mapping, None] = None, - config: dict = config, - **kwargs, - ): + def __init__(self, arg: Mapping, config: dict = config): self.config: dict = config self._record: list[tuple[str, tuple[str, ...], Any]] = [] - - if arg is not None: - if not isinstance(arg, Mapping): - raise TypeError(f"arg must be a dictionary, got {type(arg).__name__}") - for key, value in arg.items(): - key, value = check_key_val(key, value) - self._assign(key.split("."), value, config) - if kwargs: - for key, value in kwargs.items(): - key = key.replace("__", ".") - key, value = check_key_val(key, value) - self._assign(key.split("."), value, config) + for key, value in arg.items(): + _warn_if_unknown(key) + self._assign(key.split("."), value, config) def __enter__(self): return self.config @@ -151,8 +132,7 @@ def _assign( record : bool, optional Whether this operation needs to be recorded to allow for rollback. """ - key = canonical_name(keys[0], d) - + key = keys[0] path = path + (key,) if len(keys) == 1: @@ -174,214 +154,41 @@ def _assign( self._assign(keys[1:], value, d[key], path, record=record) -def refresh(config: dict = config, defaults: list[Mapping] = defaults, **kwargs) -> None: - """ - Update configuration by re-reading yaml files and env variables - - This mutates the global emdatabase.config.config, or the config parameter if - passed in. - - This goes through the following stages: - - 1. Clearing out all old configuration - 2. Updating from the stored defaults (see update_defaults) - 3. Updating from yaml files and environment variables - - See Also - -------- - emdatabase.config.collect: for parameters - emdatabase.config.update_defaults - """ +def refresh() -> None: + """Re-read the configuration: the shipped defaults, then the config files, + then the environment. Anything changed with :class:`set` is dropped.""" config.clear() + update(config, defaults) + for document in collect_yaml(_config_dir()): + for key in document: + _warn_if_unknown(key) + update(config, document) + update(config, collect_env()) # collect_env builds through set, which warns - for d in defaults: - update(config, d, priority="new") - - update(config, collect(**kwargs)) - -def get( - key: str, - default: Any = no_default, - config: dict = config, - override_with: Any = None, -) -> Any: - """ - Get elements from global config - - If ``override_with`` is not None this value will be passed straight back. - - Use '.' for nested access - """ - if override_with is not None: - return override_with - keys = key.split(".") +def get(key: str, default: Any = no_default) -> Any: + """Get a configuration value. Use '.' for nested access.""" result = config - for k in keys: - k = canonical_name(k, result) + for k in key.split("."): try: result = result[k] except (TypeError, IndexError, KeyError): if default is not no_default: return default - else: - raise + raise return result -def update_defaults(new: dict, config: dict = config, defaults: list[Mapping] = defaults) -> None: - """Add a new set of defaults to the configuration - - It does two things: - - 1. Add the defaults to a global collection to be used by refresh later - 2. Updates the global config with the new configuration - prioritizing older values over newer ones - """ - current_defaults = merge(*defaults) - # Registered before the keys are checked: they are what "known key" means. - defaults.append(new) - - for key, value in list(new.items()): - key, nval = check_key_val(key, value) - new[key] = nval - - update(config, new, priority="new-defaults", defaults=current_defaults) - - -def _initialize() -> None: - fn = os.path.join(os.path.dirname(__file__), "emdatabase.yaml") - - with open(fn) as f: - shipped = yaml.safe_load(f) - - update_defaults(shipped) - - -def canonical_name(k: str, config: dict) -> str: - """Return the canonical name for a key. - - Handles user choice of '-' or '_' conventions by standardizing on whichever - version was set first. If a key already exists in either hyphen or - underscore form, the existing version is the canonical name. If neither - version exists the original key is used as is. - """ - try: - if k in config: - return k - except TypeError: - # config is not a mapping, return the same name as provided - return k - - altk = k.replace("_", "-") if "_" in k else k.replace("-", "_") - - if altk in config: - return altk - - return k - - -def update( - old: dict, - new: Mapping, - priority: Literal["old", "new", "new-defaults"] = "new", - defaults: Mapping | None = None, - check: bool = True, -) -> dict: - """Update a nested dictionary with values from another - - This is like dict.update except that it smoothly merges nested values - - This operates in-place and modifies old - - Parameters - ---------- - priority: string {'old', 'new', 'new-defaults'} - If new (default) then the new dictionary has preference. - Otherwise the old dictionary does. - If 'new-defaults', a mapping should be given of the current defaults. - Only if a value in ``old`` matches the current default, it will be - updated with ``new``. - check: bool - Whether to run the keys through :func:`check_key_val`. False on the - recursive call, because the unknown-key warning is about top-level keys - and a location's name is not one. - - Examples - -------- - >>> a = {'x': 1, 'y': {'a': 2}} - >>> b = {'x': 2, 'y': {'b': 3}} - >>> update(a, b) # doctest: +SKIP - {'x': 2, 'y': {'a': 2, 'b': 3}} - - >>> a = {'x': 1, 'y': {'a': 2}} - >>> b = {'x': 2, 'y': {'b': 3}} - >>> update(a, b, priority='old') # doctest: +SKIP - {'x': 1, 'y': {'a': 2, 'b': 3}} - - >>> d = {'x': 0, 'y': {'a': 2}} - >>> a = {'x': 1, 'y': {'a': 2}} - >>> b = {'x': 2, 'y': {'a': 3, 'b': 3}} - >>> update(a, b, priority='new-defaults', defaults=d) # doctest: +SKIP - {'x': 1, 'y': {'a': 3, 'b': 3}} - - """ +def update(old: dict, new: Mapping) -> None: + """Merge ``new`` into ``old`` in place; ``new`` wins, and a nested mapping is + merged key by key rather than replacing the one in ``old``.""" for k, v in new.items(): - if check: - k, v = check_key_val(k, v) - k = canonical_name(k, old) - if isinstance(v, Mapping): - if k not in old or old[k] is None or not isinstance(old[k], dict): + if not isinstance(old.get(k), dict): old[k] = {} - update( - old[k], - v, - priority=priority, - defaults=defaults.get(k) if defaults else None, - check=False, - ) + update(old[k], v) else: - if ( - priority == "new" - or k not in old - or ( - priority == "new-defaults" - and defaults - and k in defaults - and defaults[k] == old[k] - ) - ): - old[k] = v - - return old - - -def collect(path: Path | str | None = None, env: Mapping[str, str] | None = None) -> dict: - """ - Collect configuration from the config directory and the environment - - Parameters - ---------- - path : Path or str, optional - Directory (or single file) to read yaml config from. Defaults to the - config directory, ``EMDATABASE_CONFIG`` or ``~/.config/emdatabase``. - - env : Mapping[str, str] - The system environment variables - - Returns - ------- - config: dict - - """ - if path is None: - path = _config_dir() - if env is None: - env = os.environ - - configs = [*collect_yaml(path=Path(path)), collect_env(env=env)] - return merge(*configs) + old[k] = v def collect_yaml(path: Path) -> Iterator[dict]: @@ -390,29 +197,18 @@ def collect_yaml(path: Path) -> Iterator[dict]: Every ``*.yaml`` and ``*.yml`` in ``path`` is parsed, in name order; a path to a single file is read as itself. """ - file_paths = [] - if path.exists(): - if path.is_dir(): - try: - file_paths.extend(path.glob("*.yaml")) - file_paths.extend(path.glob("*.yml")) - file_paths = sorted(file_paths) - except OSError: - # Ignore permission errors - pass - else: - file_paths.append(path) + if not path.exists(): + return + file_paths = sorted([*path.glob("*.yaml"), *path.glob("*.yml")]) if path.is_dir() else [path] for p in file_paths: - loaded = _load_config_file(p) - if loaded is not None: - yield loaded + yield _load_config_file(p) def collect_env(env: Mapping[str, str] | None = None) -> dict: """Collect config from environment variables This grabs environment variables of the form "EMDATABASE_FOO__BAR_BAZ=123" - and turns these into config variables of the form ``{"foo": {"bar-baz": + and turns these into config variables of the form ``{"foo": {"bar_baz": 123}}``. It transforms the key and value in the following way: - Strips the ``EMDATABASE_`` prefix and lower-cases the rest @@ -448,91 +244,37 @@ def interpret_value(value: str) -> Any: return hardcoded_map.get(value.lower(), value) -def merge(*dicts: Mapping) -> dict: - """Update a sequence of nested dictionaries - - This prefers the values in the latter dictionaries to those in the former +def _load_config_file(path: Path) -> dict: + """Parse a config file, which has to hold a mapping or nothing. - Examples - -------- - >>> a = {'x': 1, 'y': {'a': 2}} - >>> b = {'y': {'b': 3}} - >>> merge(a, b) # doctest: +SKIP - {'x': 1, 'y': {'a': 2, 'b': 3}} + A yaml syntax error already names the file and line, given the open file. """ - result: dict = {} - for d in dicts: - update(result, d, check=False) - return result - - -def _load_config_file(path: Path) -> dict | None: - """A helper for loading a config file from a path, and erroring - appropriately if the file is malformed.""" - try: - with open(path) as f: - loaded = yaml.safe_load(f.read()) - except OSError: - # Ignore permission errors - return None - except Exception as exc: - raise ValueError( - f"An emdatabase config file at {str(path)!r} is malformed, original error " - f"message:\n\n{exc}" - ) from None + with open(path) as f: + loaded = yaml.safe_load(f) if loaded is not None and not isinstance(loaded, dict): raise ValueError( f"An emdatabase config file at {str(path)!r} is malformed - config files must " f"have a dict as the top level object, got a {type(loaded).__name__} instead" ) - return loaded - - -def check_key_val(key: str, val: Any, deprecations: dict = deprecations) -> tuple[str, Any]: - """Check whether a key has been renamed, removed, or is not one we ship + return loaded or {} - A key that is none of the shipped defaults warns and is still set: config is - not a schema, and refusing an unknown key would break anything that stores - its own. - Parameters - ---------- - key : str - The configuration key to check. May be dotted, in which case only the - part before the first '.' is checked. - deprecations : Dict[str, str] - The mapping of aliases +def _warn_if_unknown(key: str) -> None: + """Warn about a key whose top level is none of the shipped ones. - Returns - ------- - new: str - The proper key, whether the original (if no deprecation) or the aliased - value + It is still set: config is not a schema, and refusing an unknown key would + break anything that stores its own. """ - if key in deprecations: - new = deprecations[key] - if new: - warnings.warn(f'Configuration key "{key}" has been deprecated. Please use "{new}" ') - else: - raise ValueError(f'Configuration value "{key}" has been removed') - - top = key.split(".")[0] - # The top-level keys of the registered defaults, not merge(*defaults): - # merge() goes through update(), which calls back into here. - known = {k for d in defaults for k in d} - if top not in known and top not in deprecations: + if key.split(".")[0] not in defaults: warnings.warn(f'Unknown configuration key "{key}"') - return key, val - -def _dump(document: Mapping, path: Path | str | None = None) -> Path: +def _dump(document: Mapping, path: Path | str | None = None) -> None: """Write ``document`` to a yaml file, creating the config directory.""" path = Path(path) if path is not None else _config_dir() / "config.yaml" path.parent.mkdir(parents=True, exist_ok=True) with open(path, "w") as f: yaml.dump(dict(document), f) - return path def write(path: Path | str | None = None) -> None: @@ -555,7 +297,7 @@ def _persist_locations(updates: Mapping[str, str | None], remove: str | None = N caller asked for. """ path = _config_dir() / "config.yaml" - document = (_load_config_file(path) if path.exists() else None) or {} + document = _load_config_file(path) if path.exists() else {} configured = document.get("locations") if not isinstance(configured, dict): configured = {} @@ -789,21 +531,15 @@ def remove_location(name_or_path: Path | str, persist: bool = True) -> None: f"{[(loc.name, str(loc.path)) for loc in locations()]}" ) - updated: dict[str, str | None] = {} - for n, p in current.items(): - if n == name: - if n == "personal": - updated[n] = None - continue - updated[n] = str(p) if p is not None else None + updated = {n: (str(p) if p is not None else None) for n, p in current.items() if n != name} if name == "personal": - updated.setdefault("personal", None) + updated["personal"] = None set({"locations": updated}) if persist: _persist_locations({"personal": None} if name == "personal" else {}, remove=name) -def first_run_notice(directory: Path | None = None) -> None: +def first_run_notice(directory: Path) -> None: """Say where downloads will go, once per process. Called from :func:`data_dir` when nothing is configured and the default @@ -814,8 +550,6 @@ def first_run_notice(directory: Path | None = None) -> None: return _NOTICE_SHOWN = True - if directory is None: - directory = Path(pooch.os_cache("emdatabase")) lines = [ f"emdatabase will download datasets to {directory}.", 'Change it with emdatabase.add_location("/somewhere/else", name="personal") or by ' @@ -844,5 +578,4 @@ def first_run_notice(directory: Path | None = None) -> None: logging.getLogger("emdatabase").info(" ".join(lines)) -_initialize() refresh() diff --git a/emdatabase/tests/test_config.py b/emdatabase/tests/test_config.py index 96e7757..d706a22 100644 --- a/emdatabase/tests/test_config.py +++ b/emdatabase/tests/test_config.py @@ -96,6 +96,19 @@ def test_a_nested_key_under_a_known_one_does_not_warn(recwarn): assert config.resolve_destination("other") == Path("/other") +def test_location_names_differing_by_hyphen_and_underscore_stay_separate(tmp_path): + _write_yaml(tmp_path, locations={"example-data": "/a", "example_data": "/b"}) + config.refresh() + config.set({"locations.group-one": "/c", "locations.group_one": "/d"}) + assert config.get("locations") == { + "personal": None, + "example-data": "/a", + "example_data": "/b", + "group-one": "/c", + "group_one": "/d", + } + + def test_write_round_trips(tmp_path): config.set({"locations": {"group": "/group", "personal": str(tmp_path / "written")}}) path = tmp_path / "config" / "config.yaml" From d0a59d576e0ec5a80fddf3b0d0ac45309fb82af9 Mon Sep 17 00:00:00 2001 From: arthurmccray Date: Mon, 14 Sep 2026 14:42:15 -0700 Subject: [PATCH 05/18] simplifying dataset --- emdatabase/catalogue.py | 2 +- emdatabase/downloadable_dataset.py | 164 ++++++++++------------------- emdatabase/tests/test_weights.py | 2 +- 3 files changed, 57 insertions(+), 111 deletions(-) diff --git a/emdatabase/catalogue.py b/emdatabase/catalogue.py index 868db6c..1b3e9d6 100644 --- a/emdatabase/catalogue.py +++ b/emdatabase/catalogue.py @@ -155,7 +155,7 @@ def entry(name: str, ds: DownloadableDataset) -> dict: "source": md.source, "file": md.file, "url": ds.download_url, - "latest_checksum": ds.latest_checksum or "", + "latest_checksum": ds.checksum or "", "versions": _versions(ds), "model_class": md.model.class_ if md.model else "", "model_framework": md.model.framework if md.model else "", diff --git a/emdatabase/downloadable_dataset.py b/emdatabase/downloadable_dataset.py index 67e9d32..2e46713 100644 --- a/emdatabase/downloadable_dataset.py +++ b/emdatabase/downloadable_dataset.py @@ -31,7 +31,6 @@ # repeating. A failed fetch caches as None, so an offline session pays the # timeout once. _UPSTREAM_CACHE: dict[str, dict[str, Any] | None] = {} -_UPSTREAM_LOCK = threading.Lock() _WARNED_STALE: set[str] = set() @@ -42,8 +41,7 @@ class StaleIndexWarning(UserWarning): def _clear_upstream_cache() -> None: """Forget the fetched index documents and which families have warned.""" - with _UPSTREAM_LOCK: - _UPSTREAM_CACHE.clear() + _UPSTREAM_CACHE.clear() _WARNED_STALE.clear() @@ -54,23 +52,17 @@ def _upstream_document(origin_filename: str) -> dict[str, Any] | None: YAML - is the same answer: nothing to compare against. An update check is not worth an exception in the middle of a download. """ - with _UPSTREAM_LOCK: - if origin_filename in _UPSTREAM_CACHE: - return _UPSTREAM_CACHE[origin_filename] - document: dict[str, Any] | None = None - try: - request = urllib.request.Request( - UPSTREAM_INDEX + origin_filename, headers={"User-Agent": USER_AGENT} - ) - with urllib.request.urlopen(request, timeout=3) as response: - parsed = yaml.safe_load(response.read()) - if isinstance(parsed, dict): - document = parsed - except Exception: - document = None - with _UPSTREAM_LOCK: - _UPSTREAM_CACHE[origin_filename] = document - return document + if origin_filename not in _UPSTREAM_CACHE: + try: + request = urllib.request.Request( + UPSTREAM_INDEX + origin_filename, headers={"User-Agent": USER_AGENT} + ) + with urllib.request.urlopen(request, timeout=3) as response: + parsed = yaml.safe_load(response.read()) + except Exception: + parsed = None + _UPSTREAM_CACHE[origin_filename] = parsed if isinstance(parsed, dict) else None + return _UPSTREAM_CACHE[origin_filename] def upstream_metadata(name: str, origin_filename: str) -> DatasetMetadata | None: @@ -82,13 +74,11 @@ def upstream_metadata(name: str, origin_filename: str) -> DatasetMetadata | None document = _upstream_document(origin_filename) if document is None: return None - spec = document.get(name) - if spec is None: - # The class name is the YAML key with its spaces and hyphens replaced. - spec = next( - (v for k, v in document.items() if str(k).replace(" ", "_").replace("-", "_") == name), - None, - ) + # The class name is the YAML key with its spaces and hyphens replaced. + spec = next( + (v for k, v in document.items() if str(k).replace(" ", "_").replace("-", "_") == name), + None, + ) if not isinstance(spec, Mapping): return None try: @@ -283,28 +273,24 @@ def total(self, value: int) -> None: from tqdm.auto import tqdm self._total = int(value or 0) - if self._bar is None: - self._bar = tqdm( - total=self._total, - desc=self._desc, - unit="B", - unit_scale=True, - # Windows terminals do not always have the box-drawing glyphs. - ascii=sys.platform == "win32", - leave=True, - ) - else: - self._bar.reset(total=self._total) + self._bar = tqdm( + total=self._total, + desc=self._desc, + unit="B", + unit_scale=True, + # Windows terminals do not always have the box-drawing glyphs. + ascii=sys.platform == "win32", + leave=True, + ) def update(self, n: int) -> None: - if self._bar is not None: - self._bar.update(n) + self._bar.update(n) def reset(self) -> None: - if self._bar is not None: - self._bar.reset(total=self._total) + self._bar.reset(total=self._total) def close(self) -> None: + # Called for a cached file too, where pooch never set a total. if self._bar is not None: self._bar.close() self._bar = None @@ -319,8 +305,8 @@ class DownloadableDataset: Everything the YAML declares is on :attr:`metadata` (``ds.metadata.technique``). The fields the download machinery itself needs are also reachable directly, as :attr:`source`, :attr:`file`, - :attr:`url`, :attr:`checksum` and :attr:`size_bytes`; the link that is - actually fetched is :attr:`download_url`. + :attr:`checksum` and :attr:`size_bytes`; the link that is actually fetched + is :attr:`download_url`. A ``kind: weights`` entry is a family rather than a single file: :attr:`versions` lists the dated snapshots it can be pinned to, and the @@ -350,10 +336,6 @@ def source(self) -> str: def file(self) -> str: return self.metadata.file - @property - def url(self) -> str | None: - return self.metadata.url - def _resolve(self, version: str | None = None) -> _Resolved: """The link, checksum and local name for one version of this entry. @@ -424,11 +406,6 @@ def versions(self) -> tuple[str, ...]: """ return tuple(sorted(self.metadata.versions, reverse=True)) - @property - def latest_checksum(self) -> str | None: - """What the ``latest`` link served when the index was written.""" - return self._resolve(None).checksum - def filename(self, version: str | None = None) -> str: """The name the file is saved under locally. @@ -564,26 +541,18 @@ def download( return DatasetPath( self._retrieve(destination, progressbar, chunk_size, version, refresh) ) - # Resolve where the file will end up: an existing copy in a shared - # location or in the personal one, otherwise the personal one. - name = self.filename(version) - if destination is not None: - target = self._resolve_destination(destination) / name - elif refresh: - target = self._resolve_destination(None) / name - else: - target = self.filepath(version) or self._resolve_destination(None) / name + # Where the file will end up: the copy the search order finds, unless a + # destination or a refresh asks for a fresh one. + existing = None if destination is not None or refresh else self.filepath(version) + target = existing or self._resolve_destination(destination) / self.filename(version) # In Jupyter (with the widget installed) a background download pops a # cancelable toast; the toast's monitor replaces the plain progress bar. monitor = finish = None if progressbar is True: # a caller's own Progress is theirs to drive - try: - from emdatabase.widget import _attach_toast + from emdatabase.widget import _attach_toast - label = type(self).__name__ + (f"@{version}" if version else "") - monitor, finish = _attach_toast(label) - except Exception: - monitor = finish = None + label = type(self).__name__ + (f"@{version}" if version else "") + monitor, finish = _attach_toast(label) progress = monitor if monitor is not None else progressbar future = _get_executor().submit( self._retrieve, destination, progress, chunk_size, version, refresh @@ -611,53 +580,35 @@ def _retrieve( resolved = self._resolve(version) newer = self._check_upstream(version, refresh) if progressbar is True: - try: - import tqdm # noqa: F401 - except ImportError: - print("`tqdm` is not installed, progress bar will be disabled.") - progressbar = False - else: - # Our own bar rather than pooch's; see _TqdmProgress. - progressbar = _TqdmProgress(resolved.file) - if destination is None: - # A refresh is about replacing your own copy, so it never reads and - # never writes a shared location. - shared = None if refresh else self._find_in_shared_locations(version) + progressbar = _TqdmProgress(resolved.file) # our own bar rather than pooch's + # A refresh is about replacing your own copy, so it never reads and + # never writes a shared location. + if destination is None and not refresh: + shared = self._find_in_shared_locations(version) if shared is not None: return shared - destination = self._resolve_destination(None) - else: - destination = self._resolve_destination(destination) - # Instantiate an Http downloader with a custom user agent - headers = {"User-Agent": USER_AGENT} + destination = self._resolve_destination(destination) downloader = pooch.HTTPDownloader( progressbar=progressbar, # pyright: ignore[reportArgumentType] chunk_size=chunk_size, - headers=headers, + headers={"User-Agent": USER_AGENT}, ) try: if refresh: # pooch keeps a file whose hash it was not given anything to # check against, so the copy has to go before it will re-fetch. - (Path(destination) / resolved.file).unlink(missing_ok=True) - if newer is not None: - filepath = pooch.retrieve( - url=newer.url, - known_hash=newer.checksum, - fname=resolved.file, - path=destination, - downloader=downloader, # pyright: ignore[reportArgumentType] - ) - elif resolved.pinned: + (destination / resolved.file).unlink(missing_ok=True) + if newer is None and not resolved.pinned: + filepath = self._retrieve_latest(resolved, destination, downloader) + else: + pin = newer or resolved # the newer link on main, if there is one filepath = pooch.retrieve( - url=resolved.url, - known_hash=resolved.checksum, + url=pin.url, + known_hash=pin.checksum, fname=resolved.file, path=destination, downloader=downloader, # pyright: ignore[reportArgumentType] ) - else: - filepath = self._retrieve_latest(resolved, Path(destination), downloader) finally: # pooch only closes the bar on the happy path, so a failed or # cancelled download would leave it hanging open. @@ -684,7 +635,7 @@ def _check_upstream(self, version: str | None, refresh: bool) -> WeightsVersion return None upstream = upstream_metadata(type(self).__name__, self._origin.name) latest = upstream.latest if upstream is not None else None - if upstream is None or latest is None or latest.checksum == self._resolve(None).checksum: + if upstream is None or latest is None or latest.checksum == self.checksum: return None if refresh: return latest @@ -744,13 +695,8 @@ def _find_in_shared_locations(self, version: str | None = None) -> Path | None: from emdatabase import config name = self.filename(version) - for location in config.locations(): - if location.kind == "personal": - continue - candidate = location.path / name - if candidate.exists(): - return candidate - return None + shared = (loc.path / name for loc in config.locations() if loc.kind != "personal") + return next((path for path in shared if path.exists()), None) def filepaths(self, version: str | None = None) -> list[Path]: """Every copy of the dataset on disk, in search order. diff --git a/emdatabase/tests/test_weights.py b/emdatabase/tests/test_weights.py index 40dfbe2..a1bda43 100644 --- a/emdatabase/tests/test_weights.py +++ b/emdatabase/tests/test_weights.py @@ -211,7 +211,7 @@ def test_the_plain_fields_describe_latest(): ds = DownloadableDataset(**ENTRY) assert ds.versions == (VERSION,) assert ds.download_url == ENTRY["latest"]["url"] - assert ds.checksum == ENTRY["latest"]["checksum"] == ds.latest_checksum + assert ds.checksum == ENTRY["latest"]["checksum"] assert ds.size_bytes == ENTRY["latest"]["size_bytes"] From 1fcd06f767ef94cf4beacd45a038f7f1afea6256 Mon Sep 17 00:00:00 2001 From: arthurmccray Date: Mon, 14 Sep 2026 15:06:37 -0700 Subject: [PATCH 06/18] cleanups --- .github/scripts/issue_to_yaml.py | 7 +- emdatabase/new_dataset.py | 15 ++-- emdatabase/tests/test_issue_route.py | 46 ++++------- emdatabase/tests/test_new_dataset.py | 110 +-------------------------- 4 files changed, 26 insertions(+), 152 deletions(-) diff --git a/.github/scripts/issue_to_yaml.py b/.github/scripts/issue_to_yaml.py index f55aaab..3c3fed2 100644 --- a/.github/scripts/issue_to_yaml.py +++ b/.github/scripts/issue_to_yaml.py @@ -168,14 +168,12 @@ def build_yaml(data): } entry["authors"], problems = parse_authors(data["Authors"]) if data["Kind"] == "weights": - entry["kind"] = "weights" model = { "class": data["Model Class"], "framework": data["Model Framework"], "quantem": data["Model quantem"], } - entry["model"] = {k: v for k, v in model.items() if v} - entry = as_weights_family(entry, data["Version Date"] or version_date()) + entry = as_weights_family(entry, data["Version Date"] or version_date(), model) return build_document(name, entry), name, problems @@ -183,6 +181,9 @@ def write_yaml(issue_file, out_dir): """Parse one issue body and write the entry it describes into ``out_dir``.""" document, dataset_name, problems = build_yaml(parse_issue_body(Path(issue_file).read_text())) out_path = Path(out_dir) / f"{dataset_name}.yaml" + if out_path.exists(): + # Anyone can open an issue; the pull request it opens adds, never replaces. + problems.append(f"{out_path} already exists; choose another --Dataset Name--") # Nothing is downloaded for an issue that is already known to be wrong. if not problems: for line in fill_download_fields(document): diff --git a/emdatabase/new_dataset.py b/emdatabase/new_dataset.py index d1f44cc..a8f99f4 100644 --- a/emdatabase/new_dataset.py +++ b/emdatabase/new_dataset.py @@ -180,10 +180,8 @@ def split_url(url: str) -> tuple[str, str, str]: and the name, which is how nearly every entry is written. One with a query string, or with no extension on its last segment, names nothing: it is kept whole as ``url``, ``source`` is the host it points at, and the file name has - to come from the server. A Google Drive share link is normalised first, so - what is written out is the link that serves the file. + to come from the server. Pass the link through :func:`normalize_url` first. """ - url = normalize_url(url) parts = urllib.parse.urlsplit(url) if not (parts.scheme and parts.netloc): return "", "", "" @@ -275,8 +273,9 @@ def version_date() -> str: return datetime.date.today().strftime("%y%m%d") -def as_weights_family(entry: dict[str, Any], date: str) -> dict[str, Any]: - """The entry with its download fields moved into ``latest`` and one version. +def as_weights_family(entry: dict[str, Any], date: str, model: dict[str, str]) -> dict[str, Any]: + """The entry as a weights family: its download fields moved into ``latest`` + and one version, and ``model``, blank fields dropped, as its model block. A weights entry is a family: ``latest`` follows the published link, and a dated version pins each state that link has served. A new entry is both, @@ -291,6 +290,8 @@ def as_weights_family(entry: dict[str, Any], date: str) -> dict[str, Any]: pin = {key: value for key, value in pin.items() if value} return { **entry, + "kind": "weights", + "model": {key: value for key, value in model.items() if value}, "url": None, "checksum": None, "size_bytes": None, @@ -522,14 +523,12 @@ def main(argv: list[str] | None = None) -> int: "authors": _ask_authors(args.yes), } if args.kind == "weights": - entry["kind"] = "weights" model = { "class": _ask("model class, e.g. quantem.ml.inr.INR", assume_yes=args.yes), "framework": _ask("framework", "torch", args.yes), "quantem": _ask('quantem versions, e.g. ">=0.2,<0.3"', assume_yes=args.yes), } - entry["model"] = {k: v for k, v in model.items() if v} - entry = as_weights_family(entry, args.version_date) + entry = as_weights_family(entry, args.version_date, model) document = build_document(name, entry) problems = validate_document(document, origin=out_path) diff --git a/emdatabase/tests/test_issue_route.py b/emdatabase/tests/test_issue_route.py index 9f7875e..1072721 100644 --- a/emdatabase/tests/test_issue_route.py +++ b/emdatabase/tests/test_issue_route.py @@ -123,15 +123,6 @@ def test_issue_form_offers_the_whole_technique_vocabulary(issue_to_yaml): assert [o["label"] for o in block["attributes"]["options"]] == list(techniques()) -def test_issue_form_asks_for_the_authors_as_one_block(issue_to_yaml): - """One required textarea, not a field per author part.""" - form = yaml.safe_load(ISSUE_FORM.read_text(encoding="utf-8")) - blocks = {b["id"]: b for b in form["body"] if b["type"] != "markdown"} - assert blocks["authors"]["type"] == "textarea" - assert blocks["authors"]["validations"]["required"] is True - assert not {"name", "affiliation", "orcid"} & set(blocks) - - def test_issue_weights_entry_validates(parse): body = _issue_body( **{ @@ -324,32 +315,21 @@ def test_issue_a_bad_author_line_stops_the_run_and_says_so(issue_to_yaml, tmp_pa assert list(out.iterdir()) == [] -# -- the size and the checksum ----------------------------------------------- - - -def test_issue_takes_the_size_it_was_given(issue_to_yaml, monkeypatch): - """A size on the issue means the server is not asked for it.""" +def test_issue_does_not_replace_an_existing_entry(issue_to_yaml, tmp_path, capsys): + """Anyone can open an issue, so a name already in the index is refused.""" + issue = tmp_path / "issue.txt" + issue.write_text(_authors_body("Jane Doe; University of Somewhere"), encoding="utf-8") + out = tmp_path / "index" + out.mkdir() + existing = out / "MgONanoCrystals.yaml" + existing.write_text("shipped", encoding="utf-8") + with pytest.raises(SystemExit): + issue_to_yaml.write_yaml(issue, out) + assert "already exists" in capsys.readouterr().out + assert existing.read_text(encoding="utf-8") == "shipped" - def no_head(url): - raise AssertionError(f"asked the server about {url}") - monkeypatch.setattr(issue_to_yaml, "content_length", no_head) - body = _issue_body( - **{ - "--Dataset Name--": "MgONanoCrystals", - "--URL--": "https://drive.google.com/uc?export=download&id=1inQ6DQ2zH40Ccd", - "--File Name--": "MgONanoCrystals.zspy", - "--Checksum--": "md5:df9376d5c020a23f0f7f51cfe79f303f", - "--Size (bytes)--": "1104287335", - "--Description--": "A 4D-STEM dataset of MgO nanocrystals.", - "--Dataset License--": "CC-BY-4.0", - "Technique": _ticked("4D-STEM"), - } - ) - document, name, problems = issue_to_yaml.build_yaml(issue_to_yaml.parse_issue_body(body)) - assert problems == [] - assert validate_document(document) == [] - assert document[name]["size_bytes"] == 1104287335 +# -- the size and the checksum ----------------------------------------------- def test_issue_asks_the_server_when_the_size_is_not_a_number(issue_to_yaml, monkeypatch): diff --git a/emdatabase/tests/test_new_dataset.py b/emdatabase/tests/test_new_dataset.py index 14c09c4..cc2e884 100644 --- a/emdatabase/tests/test_new_dataset.py +++ b/emdatabase/tests/test_new_dataset.py @@ -17,11 +17,9 @@ from emdatabase.metadata import validate_file from emdatabase.new_dataset import ( default_name, - download_md5, fill_download_fields, main, normalize_url, - split_url, version_date, write_document, ) @@ -138,7 +136,7 @@ def test_prompts_fill_in_the_optional_fields(server, tmp_path, monkeypatch): monkeypatch, "MyData", # entry name "A 4D-STEM dataset of something.", # description - "4D-STEM", # technique + "In-situ, 4D-STEM", # techniques, comma separated "CC-BY-4.0", # license "Direct Electron", # detector manufacturer "DE-16", # detector @@ -158,7 +156,7 @@ def test_prompts_fill_in_the_optional_fields(server, tmp_path, monkeypatch): path = tmp_path / "MyData.yaml" assert validate_file(path) == [] entry = _document(path)["MyData"] - assert entry["technique"] == ["4D-STEM"] + assert entry["technique"] == ["In-situ", "4D-STEM"] assert entry["voltage"] == "200 kV" assert entry["tags"] == ["Amorphous", "Strain"] assert entry["authors"] == { @@ -168,31 +166,6 @@ def test_prompts_fill_in_the_optional_fields(server, tmp_path, monkeypatch): assert "doi" not in entry -def test_the_technique_prompt_takes_a_comma_separated_list(server, tmp_path, monkeypatch): - base, _ = server - _answers( - monkeypatch, - "MyData", # entry name - "An in-situ 4D-STEM dataset of something.", # description - "In-situ, 4D-STEM", # techniques - "", # license - "", # detector manufacturer - "", # detector - "", # microscope vendor - "", # microscope model - "", # voltage - "", # camera length - "", # DOI - "", # tags - "", # no authors - ) - assert main([f"{base}/MyData.zspy", "--out", str(tmp_path)]) == 0 - - path = tmp_path / "MyData.yaml" - assert validate_file(path) == [] - assert _document(path)["MyData"]["technique"] == ["In-situ", "4D-STEM"] - - def _weights_answers(monkeypatch, name="DemoNet"): """The prompts a ``--kind weights`` run asks, in order.""" _answers( @@ -262,27 +235,6 @@ def test_a_version_date_that_is_not_yymmdd_is_refused(server, tmp_path, capsys): assert not list(tmp_path.glob("*.yaml")) -def test_a_dataset_entry_says_nothing_about_a_model(server, tmp_path): - base, _ = server - assert ( - main( - [ - f"{base}/MyData.zspy", - "--name", - "MyData", - "--out", - str(tmp_path), - "--description", - "A 4D-STEM dataset of something.", - "--yes", - ] - ) - == 0 - ) - entry = _document(tmp_path / "MyData.yaml")["MyData"] - assert entry["kind"] == "dataset" and "model" not in entry - - def test_a_technique_outside_the_vocabulary_is_asked_for_again( server, tmp_path, monkeypatch, capsys ): @@ -470,44 +422,6 @@ def test_a_link_that_is_not_a_drive_share_link_is_left_alone(url): def test_a_zenodo_copy_link_loses_its_download_flag(): bare = "https://zenodo.org/records/22311217/files/best.pth" assert normalize_url(bare + "?download=1") == bare - assert split_url(bare + "?download=1") == ( - "https://zenodo.org/records/22311217/files", - "best.pth", - "", - ) - - -def test_split_url_normalises_a_drive_share_link(): - """The CLI and the issue route get the rewrite for free, through split_url.""" - source, filename, link = split_url(f"https://drive.google.com/file/d/{DRIVE_ID}/view") - assert (source, filename, link) == ("https://drive.google.com", "", DRIVE_DOWNLOAD) - - -def test_write_document_writes_the_header_and_makes_the_directory(tmp_path): - """The CI script reuses this to rewrite a family file.""" - path = tmp_path / "index" / "MyData.yaml" - document = {"MyData": {"description": "d", "file": "f"}} - write_document(path, document) - text = path.read_text(encoding="utf-8") - assert text.startswith("# $schema: ./json-schema.json\n") - assert yaml.safe_load(text) == document - - -def test_download_md5_reports_the_content_type(server, tmp_path): - """A host that answers a download link with a page, not the file.""" - base, served = server - (served / "scan.html").write_text("virus scan warning", encoding="utf-8") - _, _, name, content_type = download_md5( - f"{base}/scan.html", tmp_path / "scan", progressbar=False - ) - assert content_type.startswith("text/html") - assert name == "" - - digest, size, name, content_type = download_md5( - f"{base}/uc?export=download&id=MyData.zspy", tmp_path / "data", progressbar=False - ) - assert (digest, size, name) == (MD5, len(CONTENT), "MyData.zspy") - assert content_type == "application/octet-stream" def test_the_temporary_download_does_not_stay_behind(server, tmp_path, monkeypatch): @@ -596,16 +510,6 @@ def _entry(base, **extra): } -def test_fill_download_fields_fills_in_both(server): - base, _ = server - document = _entry(base) - lines = fill_download_fields(document) - assert document["MyData"]["checksum"] == f"md5:{MD5}" - assert document["MyData"]["size_bytes"] == len(CONTENT) - assert len(lines) == 1 - assert f"md5:{MD5}" in lines[0] and str(len(CONTENT)) in lines[0] - - def test_fill_download_fields_leaves_what_is_already_there(server): """Only the missing field is written; a checksum given by hand is not second-guessed.""" base, _ = server @@ -670,13 +574,3 @@ def test_fill_download_fields_follows_every_weights_pin(server): assert [line.split(":")[0] for line in lines] == ["DemoNet latest", "DemoNet version 260101"] # Nothing is written at the top level, where a family may not carry them. assert not {"checksum", "size_bytes"} & set(entry) - - -def test_fill_download_fields_refuses_a_page(server): - """A Drive viewer page, or a 404 dressed up as HTML, is not the file.""" - base, served = server - (served / "scan.html").write_text("virus scan warning", encoding="utf-8") - document = _entry(base, file="scan.html") - with pytest.raises(ValueError, match="served a page rather than the file"): - fill_download_fields(document) - assert "checksum" not in document["MyData"] From 867391257570e81a86ef7814245ca0b48e18e5ee Mon Sep 17 00:00:00 2001 From: arthurmccray Date: Mon, 14 Sep 2026 15:21:19 -0700 Subject: [PATCH 07/18] simplifications --- .github/scripts/check_latest_weights.py | 107 +++++++----------------- .github/scripts/fill_download_fields.py | 7 +- .github/workflows/check_weights.yml | 2 +- emdatabase/metadata.py | 6 +- emdatabase/tests/test_check_weights.py | 32 ------- 5 files changed, 37 insertions(+), 117 deletions(-) diff --git a/.github/scripts/check_latest_weights.py b/.github/scripts/check_latest_weights.py index 5448f11..c2e8264 100644 --- a/.github/scripts/check_latest_weights.py +++ b/.github/scripts/check_latest_weights.py @@ -36,7 +36,6 @@ import argparse import datetime import json -import os import re import shutil import subprocess @@ -51,7 +50,6 @@ from emdatabase.metadata import ( INDEX_DIR, - NON_DATASET_FILES, DatasetMetadata, WeightsVersion, dataset_files, @@ -77,11 +75,6 @@ def asset_url(tag: str, asset: str) -> str: return f"{RELEASE_DOWNLOADS}/{tag}/{asset}" -def is_archived(url: str, tag: str) -> bool: - """Whether a version's link already points into the archive release.""" - return url.startswith(f"{RELEASE_DOWNLOADS}/{tag}/") - - @dataclass(frozen=True) class ZenodoLink: """A Zenodo record file link, split into the pieces the API needs.""" @@ -118,23 +111,9 @@ def fetch_json(url: str) -> dict[str, Any]: return json.loads(response.read().decode("utf-8")) -def index_files(index_dir: Path | None) -> list[Path]: - """Every dataset YAML to check, sorted by name.""" - if index_dir is None: - return dataset_files() - return sorted(p for p in index_dir.rglob("*.y*ml") if p.name not in NON_DATASET_FILES) - - def run_gh(args: list[str], *, check: bool = True) -> subprocess.CompletedProcess[str]: - """Run one ``gh`` command; the only place this script shells out. - - ``gh`` reads ``GH_TOKEN``, while a workflow hands the job ``GITHUB_TOKEN``. - """ - env = os.environ.copy() - token = env.get("GH_TOKEN") or env.get("GITHUB_TOKEN") - if token: - env["GH_TOKEN"] = token - return subprocess.run(["gh", *args], check=check, capture_output=True, text=True, env=env) + """Run one ``gh`` command; the only place this script shells out.""" + return subprocess.run(["gh", *args], check=check, capture_output=True, text=True) def upload_asset(tag: str, path: Path, asset: str) -> None: @@ -212,9 +191,7 @@ def check_family( """Download one family's ``latest`` and update ``entry`` in place.""" report = Report(lines=[f"### {name}"]) latest = metadata.latest - if latest is None: - report.lines.append("- no `latest` link, so there is nothing to follow") - return report + assert latest is not None # the schema requires one of every weights entry link = zenodo_link(latest.url) if link is not None: @@ -250,19 +227,6 @@ def check_family( return report -def _newest_record(link: ZenodoLink, record: dict[str, Any]) -> dict[str, Any]: - """The newest record of this concept. - - ``links.latest`` is what Zenodo publishes for it; the concept record id is - the fallback, which the API resolves to the same place. - """ - follow = (record.get("links") or {}).get("latest") - if not follow: - concept = record.get("conceptrecid") - follow = f"{link.api}/{concept}" if concept else None - return fetch_json(str(follow)) if follow else record - - def _record_file(record: dict[str, Any], key: str) -> dict[str, Any] | None: """The file entry named ``key``, or the only one when the name has changed.""" files = [item for item in record.get("files") or [] if isinstance(item, dict)] @@ -272,13 +236,18 @@ def _record_file(record: dict[str, Any], key: str) -> dict[str, Any] | None: return files[0] if len(files) == 1 else None -def _record_date(record: dict[str, Any]) -> str: - """The record's publication date as ``YYMMDD``, or today's if it has none.""" - published = str((record.get("metadata") or {}).get("publication_date", "")) - try: - return datetime.date.fromisoformat(published[:10]).strftime("%y%m%d") - except ValueError: - return version_date() +def _date_taken(report: Report, metadata: DatasetMetadata, date: str, checksum: str) -> bool: + """Whether version ``date`` already holds a different file; a failure if it does.""" + existing = metadata.versions.get(date) + if existing is None or existing.checksum == checksum: + return False + report.ok = False + report.lines.append( + f"- **version `{date}` already exists with `{existing.checksum}`**, and the newest file " + f"is `{checksum}`. Two states of this file share one date; refile the earlier one by " + "hand under the date it was published." + ) + return True def _check_zenodo( @@ -295,13 +264,15 @@ def _check_zenodo( version written for it points straight at that record. """ try: - newest = _newest_record(link, fetch_json(f"{link.api}/{link.record_id}")) - except (OSError, ValueError) as error: + record = fetch_json(f"{link.api}/{link.record_id}") + newest = fetch_json(record["links"]["latest"]) + new_id = str(newest["id"]) + published = datetime.date.fromisoformat(newest["metadata"]["publication_date"][:10]) + except (OSError, KeyError, ValueError) as error: report.ok = False report.lines.append(f"- **could not read the Zenodo API** for {latest.url}: {error}") return - new_id = str(newest.get("id", "")) served = _record_file(newest, link.key) if served is None: report.ok = False @@ -326,15 +297,8 @@ def _check_zenodo( report.lines.append(f"- unchanged; Zenodo record {new_id} is still the latest version") return - date = _record_date(newest) - existing = metadata.versions.get(date) - if existing is not None and existing.checksum != checksum: - report.ok = False - report.lines.append( - f"- **version `{date}` already exists with `{existing.checksum}`**, and Zenodo record " - f"{new_id} serves `{checksum}`. Two states of this file share one date; file the " - "earlier one under the date it was published." - ) + date = published.strftime("%y%m%d") + if _date_taken(report, metadata, date, checksum): return url = link.file_url(new_id, str(served.get("key") or link.key)) report.lines.append( @@ -342,12 +306,9 @@ def _check_zenodo( f"{link.record_id}, and the new record serves `{checksum}` " f"({format_size(size_bytes)}), filed as version `{date}`" ) - entry["latest"] = {"url": url, "checksum": checksum, "size_bytes": size_bytes} - entry.setdefault("versions", {})[date] = { - "url": url, - "checksum": checksum, - "size_bytes": size_bytes, - } + pin = {"url": url, "checksum": checksum, "size_bytes": size_bytes} + entry["latest"] = pin + entry["versions"][date] = dict(pin) # a copy, or the YAML would use an anchor report.changed = True @@ -368,7 +329,7 @@ def _backfill( date for date, version in sorted(metadata.versions.items()) if version.checksum == served.checksum - and not is_archived(version.url, options.archive_tag) + and not version.url.startswith(asset_url(options.archive_tag, "")) and zenodo_link(version.url) is None ] if not unarchived: @@ -393,14 +354,7 @@ def _new_version( ) -> None: """Record what the link serves now as a version dated today.""" date = version_date() - existing = metadata.versions.get(date) - if existing is not None and existing.checksum != served.checksum: - report.ok = False - report.lines.append( - f"- **version `{date}` already exists with `{existing.checksum}`**, and the link now " - f"serves `{served.checksum}`. Two states of this file share one date; archive the " - "earlier one by hand and file it under the date it was published." - ) + if _date_taken(report, metadata, date, served.checksum): return report.lines.append( f"- the file changed: the index has `{latest.checksum}` and the link now serves " @@ -410,7 +364,7 @@ def _new_version( _archive(report, options, served, asset) entry["latest"]["checksum"] = served.checksum entry["latest"]["size_bytes"] = served.size_bytes - entry.setdefault("versions", {})[date] = { + entry["versions"][date] = { "url": asset_url(options.archive_tag, asset), "checksum": served.checksum, "size_bytes": served.size_bytes, @@ -464,6 +418,7 @@ def _parser() -> argparse.ArgumentParser: parser.add_argument( "--index", type=Path, + default=INDEX_DIR, help=f"directory of dataset YAML to check (default {INDEX_DIR})", ) parser.add_argument("--summary", type=Path, help="write a markdown report of the run here") @@ -490,12 +445,10 @@ def main(argv: list[str] | None = None) -> int: lines: list[str] = [] ok = True - for path in index_files(args.index): + for path in dataset_files(args.index): report = check_file(path, options) ok &= report.ok lines += report.lines - if not lines: - lines = ["No weights families in the index."] summary = "\n".join(lines) print(summary) diff --git a/.github/scripts/fill_download_fields.py b/.github/scripts/fill_download_fields.py index 954e626..aa0f6d5 100644 --- a/.github/scripts/fill_download_fields.py +++ b/.github/scripts/fill_download_fields.py @@ -30,7 +30,7 @@ from emdatabase.new_dataset import build_document, fill_download_fields, write_document -def index_files(index_dir: Path | None, paths: list[Path]) -> list[Path]: +def index_files(index_dir: Path, paths: list[Path]) -> list[Path]: """Every dataset YAML to fill in: the ones named, or a whole directory. ``vendors.yaml`` and the rest of ``index/`` are not dataset collections, so @@ -40,9 +40,7 @@ def index_files(index_dir: Path | None, paths: list[Path]) -> list[Path]: if paths: # A pull request that removes an entry names a file that is gone. return [path for path in paths if path.name not in NON_DATASET_FILES and path.exists()] - if index_dir is None: - return dataset_files() - return sorted(p for p in index_dir.rglob("*.y*ml") if p.name not in NON_DATASET_FILES) + return dataset_files(index_dir) def fill_file(path: Path) -> tuple[list[str], bool]: @@ -80,6 +78,7 @@ def _parser() -> argparse.ArgumentParser: parser.add_argument( "--index", type=Path, + default=INDEX_DIR, help=f"directory of dataset YAML to fill in (default {INDEX_DIR})", ) parser.add_argument("--summary", type=Path, help="write a markdown report of the run here") diff --git a/.github/workflows/check_weights.yml b/.github/workflows/check_weights.yml index 3899318..c271098 100644 --- a/.github/workflows/check_weights.yml +++ b/.github/workflows/check_weights.yml @@ -29,7 +29,7 @@ jobs: - name: Follow each latest link env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | python .github/scripts/check_latest_weights.py \ --summary changes.md --keep-dir oversize diff --git a/emdatabase/metadata.py b/emdatabase/metadata.py index 80da5ed..7a05402 100644 --- a/emdatabase/metadata.py +++ b/emdatabase/metadata.py @@ -48,9 +48,9 @@ _STR_WIDTH = 88 -def dataset_files() -> list[Path]: - """Every dataset collection YAML, sorted by name.""" - return sorted(p for p in INDEX_DIR.rglob("*.y*ml") if p.name not in NON_DATASET_FILES) +def dataset_files(directory: Path = INDEX_DIR) -> list[Path]: + """Every dataset collection YAML in ``directory``, sorted by name.""" + return sorted(p for p in directory.rglob("*.y*ml") if p.name not in NON_DATASET_FILES) class IndexEntry(NamedTuple): diff --git a/emdatabase/tests/test_check_weights.py b/emdatabase/tests/test_check_weights.py index 6d89c33..67e3013 100644 --- a/emdatabase/tests/test_check_weights.py +++ b/emdatabase/tests/test_check_weights.py @@ -125,24 +125,6 @@ def _entry(path): return yaml.safe_load(path.read_text(encoding="utf-8"))["DemoNet"] -def test_an_unarchived_version_is_uploaded_and_its_url_rewritten(script, gh, unchanged, tmp_path): - _, _, directory, path = unchanged - code, summary = _run(script, directory, tmp_path) - - assert code == 0 - entry = _entry(path) - assert entry["versions"][OLD_DATE]["url"] == ( - "https://github.com/electronmicroscopy/emdatabase/releases/download/" - f"weights-archive/DemoNet_{OLD_DATE}.pt" - ) - assert entry["latest"]["checksum"] == _md5(LATEST_BYTES) - upload = [call for call in gh if call[:2] == ["release", "upload"]] - assert len(upload) == 1 - assert upload[0][2] == "weights-archive" - assert Path(upload[0][3]).name == f"DemoNet_{OLD_DATE}.pt" - assert f"DemoNet_{OLD_DATE}.pt" in summary.read_text() - - def test_an_already_archived_version_is_left_alone(script, gh, unchanged, tmp_path): base, _, directory, path = unchanged archived = { @@ -352,20 +334,6 @@ def test_a_new_zenodo_record_becomes_a_dated_version(script, gh, zenodo, tmp_pat assert _md5(LATEST_BYTES) in report and _md5(NEW_BYTES) in report -def test_a_new_zenodo_record_is_followed_without_links_latest(script, gh, zenodo, tmp_path): - base, served, directory, path, publish = zenodo - record = publish(NEW_RECORD, "2026-03-04", [_zenodo_file(base, NEW_RECORD, FILE, NEW_BYTES)]) - current = json.loads((served / "api" / "records" / OLD_RECORD).read_text(encoding="utf-8")) - current["links"] = {} - (served / "api" / "records" / OLD_RECORD).write_text(json.dumps(current), encoding="utf-8") - assert record["conceptrecid"] == CONCEPT - - code, _ = _run(script, directory, tmp_path) - - assert code == 0 - assert _entry(path)["versions"][NEW_DATE]["checksum"] == _md5(NEW_BYTES) - - def test_a_zenodo_record_without_the_named_file_fails(script, gh, zenodo, tmp_path): base, _, directory, path, publish = zenodo publish( From 545ae6f9e08768d940bf0a5ff6af9fbb1687fa7c Mon Sep 17 00:00:00 2001 From: arthurmccray Date: Mon, 14 Sep 2026 15:44:29 -0700 Subject: [PATCH 08/18] pruning tests --- emdatabase/tests/test_weights.py | 72 +++++--------------------------- 1 file changed, 10 insertions(+), 62 deletions(-) diff --git a/emdatabase/tests/test_weights.py b/emdatabase/tests/test_weights.py index a1bda43..48e0ad6 100644 --- a/emdatabase/tests/test_weights.py +++ b/emdatabase/tests/test_weights.py @@ -34,7 +34,6 @@ ModelInfo, WeightsVersion, format_size, - load_schema, validate_document, versioned_filename, ) @@ -127,10 +126,6 @@ def in_the_index(monkeypatch): # -- the schema ------------------------------------------------------------- -def test_a_weights_entry_validates(): - assert validate_document({"DemoNet": ENTRY}) == [] - - @pytest.mark.parametrize("missing", ["model", "latest", "versions"]) def test_weights_without_the_blocks_they_need_are_rejected(missing): entry = {k: v for k, v in ENTRY.items() if k != missing} @@ -186,14 +181,6 @@ def test_an_unquoted_version_key_is_reported(): assert any("must be quoted" in problem for problem in problems) -def test_the_model_schema_and_the_dataclass_agree(): - """``class`` is a keyword, so the field is ``class_``; nothing else differs.""" - entry_schema = next(iter(load_schema()["patternProperties"].values())) - model_schema = entry_schema["properties"]["model"] - assert list(model_schema["properties"]) == ["class", "framework", "quantem"] - assert model_schema["required"] == ["class", "framework"] - - # -- the record ------------------------------------------------------------- @@ -215,12 +202,6 @@ def test_the_plain_fields_describe_latest(): assert ds.size_bytes == ENTRY["latest"]["size_bytes"] -def test_a_dated_copy_is_named_after_its_date(): - ds = DownloadableDataset(**ENTRY) - assert ds.filename() == "DemoNet.pt" - assert ds.filename(VERSION) == "DemoNet_260902.pt" - - def test_an_unknown_version_says_which_ones_there_are(): with pytest.raises(ValueError, match=f"no version '250101'.*{VERSION}"): DownloadableDataset(**ENTRY).filename("250101") @@ -343,13 +324,9 @@ def test_a_dated_download_in_the_background(family, tmp_path): # -- finding them ----------------------------------------------------------- -def test_list_weights_and_filter_find_it(in_the_index): +def test_kind_separates_weights_from_datasets(in_the_index): assert in_the_index in [type(ds).__name__ for ds in emdatabase.list_weights()] assert in_the_index in [type(ds).__name__ for ds in emdatabase.filter(kind="weights")] - assert in_the_index in [type(ds).__name__ for ds in emdatabase.filter(version=VERSION)] - - -def test_list_datasets_still_returns_everything(in_the_index): names = [type(ds).__name__ for ds in emdatabase.list_datasets()] assert in_the_index in names assert len(names) == len(emdatabase.list_datasets(kind="dataset")) + len( @@ -380,7 +357,9 @@ def test_the_catalogue_can_be_asked_for_one_kind(in_the_index): assert in_the_index not in [it["name"] for g in datasets["groups"] for it in g["items"]] -def test_the_catalogue_row_carries_the_model(in_the_index): +def test_the_catalogue_row_carries_the_model_and_the_whole_family(in_the_index): + """The top-level link and checksum are latest; every dated version is a row + of its own, because they are downloaded and deleted one at a time.""" ds = catalogue.resolve(in_the_index) assert ds is not None row = catalogue.entry(in_the_index, ds) @@ -388,14 +367,6 @@ def test_the_catalogue_row_carries_the_model(in_the_index): assert row["model_class"] == MODEL["class"] assert row["model_framework"] == "torch" assert row["model_quantem"] == ">=0.2,<0.3" - - -def test_the_catalogue_row_carries_the_whole_family(in_the_index): - """The top-level link and checksum are latest; every dated version is a row - of its own, because they are downloaded and deleted one at a time.""" - ds = catalogue.resolve(in_the_index) - assert ds is not None - row = catalogue.entry(in_the_index, ds) assert row["url"] == ENTRY["latest"]["url"] assert row["latest_checksum"] == ENTRY["latest"]["checksum"] pin = ENTRY["versions"][VERSION] @@ -412,14 +383,6 @@ def test_the_catalogue_row_carries_the_whole_family(in_the_index): ] -def test_a_dataset_row_has_no_versions(): - ds = catalogue.resolve("CuZnHAADF") - assert ds is not None - row = catalogue.entry("CuZnHAADF", ds) - assert row["versions"] == [] - assert row["latest_checksum"] == ds.checksum - - def test_the_catalogue_row_tracks_each_version_on_disk(in_the_index, tmp_path): config.set({"locations": {"personal": str(tmp_path)}}) ds = catalogue.resolve(in_the_index) @@ -505,32 +468,24 @@ def test_an_index_that_agrees_says_nothing(upstream, tmp_path): assert path.is_file() -def test_newer_weights_upstream_warn(upstream, tmp_path): +def test_newer_weights_upstream_warn_once_per_family(upstream, tmp_path): cls, write, spec, _ = upstream write(_retrained(spec)) ds = cls() with pytest.warns(StaleIndexWarning, match="refresh=True") as record: ds.download(destination=tmp_path, progressbar=False, background=False) assert NEW_VERSION in str(record[0].message) + # A notebook loop asks about the same weights over and over; once is enough. + with warnings.catch_warnings(): + warnings.simplefilter("error", StaleIndexWarning) + ds.download(destination=tmp_path, progressbar=False, background=False) # The question is about the index, not the disk, so a copy already - # downloaded is no reason not to ask it. + # downloaded is no reason not to ask it again once the process forgets. _clear_upstream_cache() with pytest.warns(StaleIndexWarning): ds.download(destination=tmp_path, progressbar=False, background=False) -def test_the_warning_is_once_per_family(upstream, tmp_path): - """A notebook loop asks about the same weights over and over.""" - cls, write, spec, _ = upstream - write(_retrained(spec)) - ds = cls() - with pytest.warns(StaleIndexWarning): - ds.download(destination=tmp_path, progressbar=False, background=False) - with warnings.catch_warnings(): - warnings.simplefilter("error", StaleIndexWarning) - ds.download(destination=tmp_path, progressbar=False, background=False) - - def test_refresh_fetches_the_weights_the_index_on_main_names(upstream, tmp_path): cls, write, spec, served = upstream retrained = served / "DemoNet_new.pt" @@ -586,13 +541,6 @@ def test_check_updates_off_asks_nothing(upstream, asked, tmp_path): assert asked == [] -def test_a_pinned_version_asks_nothing(upstream, asked, tmp_path): - cls, write, spec, _ = upstream - write(_retrained(spec)) - cls().download(destination=tmp_path, progressbar=False, background=False, version=VERSION) - assert asked == [] - - def test_refresh_refetches_a_dated_version(upstream, asked, tmp_path): cls, _, spec, _ = upstream ds = cls() From 7dd0d5f603006aab42b1a614ef04460a46052ba0 Mon Sep 17 00:00:00 2001 From: arthurmccray Date: Mon, 14 Sep 2026 16:02:50 -0700 Subject: [PATCH 09/18] cleaning up widget --- emdatabase/tests/test_widget.py | 67 ------- emdatabase/widget.py | 329 ++++++++++++++------------------ 2 files changed, 138 insertions(+), 258 deletions(-) diff --git a/emdatabase/tests/test_widget.py b/emdatabase/tests/test_widget.py index bfdb967..b042be5 100644 --- a/emdatabase/tests/test_widget.py +++ b/emdatabase/tests/test_widget.py @@ -5,8 +5,6 @@ download is marked ``slow``. """ -import threading - import pytest import emdatabase @@ -24,13 +22,6 @@ def test_catalogue_groups_and_orders_by_technique(): cat = catalogue.catalogue() assert cat["n_total"] > 0 assert cat["groups"], "expected at least one technique group" - techniques = [g["technique"] for g in cat["groups"]] - # Alphabetical, case-insensitive, with "Other" and the weights heading last. - named = [t for t in techniques if t not in ("Other", catalogue.WEIGHTS_GROUP)] - assert named == sorted(named, key=str.lower) - assert techniques[len(named) :] == [ - t for t in ("Other", catalogue.WEIGHTS_GROUP) if t in techniques - ] # n_total counts datasets, and the groups overlap, so it is the number of # distinct names across them. names = {it["name"] for g in cat["groups"] for it in g["items"]} @@ -102,15 +93,6 @@ def test_catalogue_entry_reports_where_the_file_came_from(tmp_path): assert catalogue.entry(TINY_DATASET, ds)["location"] == "group" -def test_catalogue_downloaded_flag_tracks_the_file(tmp_path): - config.add_location(tmp_path, name="personal", persist=False) - ds = catalogue.resolve(TINY_DATASET) - assert ds is not None - assert catalogue.entry(TINY_DATASET, ds)["downloaded"] is False - (tmp_path / ds.file).write_bytes(b"x") # pretend it is downloaded - assert catalogue.entry(TINY_DATASET, ds)["downloaded"] is True - - # --------------------------------------------------------------------------- # widget # --------------------------------------------------------------------------- @@ -129,16 +111,6 @@ def test_browse_returns_widget_populated_from_the_catalogue(): assert isinstance(widget.data_dir, str) and widget.data_dir -def test_progress_trait_plumbing(): - widget = _browser() - widget._set_progress("tok", "Foo", 30, 100) - assert widget.downloads["tok"] == {"label": "Foo", "done": 30, "total": 100} - widget._set_error("tok", "Foo", "boom") - assert widget.downloads["tok"]["error"] == "boom" - widget._clear_progress("tok") - assert "tok" not in widget.downloads - - def test_command_trait_routes_to_actions(monkeypatch): """Setting the `_command` trait (what the frontend does) dispatches.""" widget = _browser() @@ -165,16 +137,6 @@ def test_command_trait_routes_to_actions(monkeypatch): ] -def test_widget_does_not_shadow_ipywidgets_comm_handler(): - """`_handle_msg` is ipywidgets' internal comm callback - overriding it breaks - all comm handling (trait sync included). The widget must not define one, so - the inherited handler stays intact.""" - widget = _browser() - assert "_handle_msg" not in type(widget).__dict__ - # It resolves to ipywidgets' Widget, not our subclass. - assert type(widget)._handle_msg.__qualname__.split(".")[0] != type(widget).__name__ - - def test_command_update_routes_through_real_comm_handler(monkeypatch): """A _command state-update (what save_changes sends) dispatches without the TypeError that a shadowed _handle_msg used to raise.""" @@ -203,17 +165,6 @@ def test_search_blob_includes_authors_and_affiliation(): assert "4d-stem" in row["search"] # technique -def test_delete_removes_downloaded_file(tmp_path): - config.add_location(tmp_path, name="personal", persist=False) - ds = catalogue.resolve(TINY_DATASET) - assert ds is not None - (tmp_path / ds.file).write_bytes(b"x") - assert ds.filepath() is not None - assert ds.delete() is True - assert ds.filepath() is None - assert ds.delete() is False # nothing left to delete - - def test_a_dated_download_passes_the_version_and_names_the_toast(monkeypatch): """The toast label is what the frontend matches a running download to, so a dated one says which version it is.""" @@ -233,14 +184,6 @@ def download(self, progressbar, background=True, version=None): assert seen == {"version": "260902", "label": "DemoNet@260902"} -def test_cancel_sets_the_event(): - widget = _browser() - event = threading.Event() - widget._cancels["tok"] = event - widget._cancel("tok") - assert event.is_set() - - def test_widget_quiets_pooch_download_logs(): """Creating a widget silences pooch's noisy "Downloading data from" INFO logs (they render as red output in Jupyter).""" @@ -297,16 +240,6 @@ def _boom(_dataset): assert "text/plain" in bundle -def test_notebook_detection_and_colab_enable_are_safe(): - """The frontend helpers must be no-ops off a notebook (e.g. under pytest), - so nothing breaks when emdatabase is imported in plain Python.""" - import emdatabase.widget as widget_mod - - assert widget_mod._in_notebook() is False - widget_mod._enable_colab_widgets() # must not raise when not on Colab - widget_mod._prepare_frontend() # idempotent, safe - - def test_attach_toast_is_noop_outside_jupyter(): """Outside a Jupyter kernel there is no toast, so a bare download is unaffected.""" diff --git a/emdatabase/widget.py b/emdatabase/widget.py index ce934b4..e4d5747 100644 --- a/emdatabase/widget.py +++ b/emdatabase/widget.py @@ -13,12 +13,16 @@ from __future__ import annotations +import functools import itertools +import logging import threading import time import warnings from pathlib import Path +import pooch + from emdatabase import catalogue as _catalogue from emdatabase.downloadable_dataset import _get_executor @@ -29,29 +33,6 @@ _PROGRESS_INTERVAL = 0.15 _pooch_quieted = False - - -def _quiet_pooch(): - """Silence pooch's "Downloading data from …" INFO logs. - - In Jupyter those propagate to stderr and render as red output. The widget - shows the same information as a toast, so once a widget is in use we don't - need pooch's chatter. Warnings and errors are left intact. Idempotent. - """ - global _pooch_quieted - if _pooch_quieted: - return - try: - import logging - - import pooch - - pooch.get_logger().setLevel(logging.WARNING) - _pooch_quieted = True - except Exception: - pass - - _colab_enabled = False @@ -74,8 +55,17 @@ def _enable_colab_widgets(): def _prepare_frontend(): - """Everything that should happen before a widget renders.""" - _quiet_pooch() + """Everything that should happen before a widget renders. + + pooch's "Downloading data from ..." INFO logs render as red output in + Jupyter, and the widget shows the same thing as a toast, so they are + silenced - once, so a level set afterwards is left alone. Warnings and + errors still come through. + """ + global _pooch_quieted + if not _pooch_quieted: + pooch.get_logger().setLevel(logging.WARNING) + _pooch_quieted = True _enable_colab_widgets() @@ -153,23 +143,29 @@ def _emit(self, force=False): self._widget._set_progress(self.token, self.label, self._done, self._total) -def _make_browser_class(): - """Build the ``DatasetBrowser`` class, importing anywidget lazily.""" +def _delete_file(dataset, name, version): + """Delete a dataset's downloaded file, warning rather than raising if it cannot be.""" + try: + dataset.delete(version=version) + except OSError as error: # read-only dir, permissions, a vanished file + warnings.warn(f"could not delete {name}: {error}", stacklevel=3) + + +@functools.cache +def _make_downloads_class(): + """Build the base of the browser and the toasts widget, importing anywidget lazily. + + Both draw one toast per running download from ``downloads``, which maps a + per-download token to ``{label, done, total}`` while it runs and to + ``{label, error}`` once it has failed, and both take the cancel and dismiss + commands those toasts send. + """ import anywidget import traitlets - class DatasetBrowser(anywidget.AnyWidget): - """Interactive, hoverable list of the emdatabase datasets.""" - - _esm = _STATIC / "browser.js" + class DownloadsWidget(anywidget.AnyWidget): _css = _STATIC / "browser.css" - # State the frontend renders. `downloads` maps a per-download token to - # {label, done, total} while running, or {label, error} on failure. - groups = traitlets.List().tag(sync=True) - data_dir = traitlets.Unicode().tag(sync=True) - n_downloaded = traitlets.Int().tag(sync=True) - n_total = traitlets.Int().tag(sync=True) downloads = traitlets.Dict().tag(sync=True) # Commands from the frontend arrive as a synced dict (more reliable than # custom comm messages): the frontend bumps a nonce so repeats register. @@ -179,82 +175,42 @@ def __init__(self, **kwargs): super().__init__(**kwargs) self._lock = threading.RLock() self._cancels: dict[str, threading.Event] = {} + self._labels: dict[str, str] = {} self._counter = itertools.count() - self.refresh() self.observe(self._on_command, names="_command") - # -- catalogue state ------------------------------------------------ - def refresh(self): - """Rebuild the catalogue - re-checks which files are on disk.""" - cat = _catalogue.catalogue() - self.data_dir = cat["data_dir"] - self.groups = cat["groups"] - self.n_downloaded = cat["n_downloaded"] - self.n_total = cat["n_total"] - - # -- commands from the frontend ------------------------------------ # NB: do NOT name a method `_handle_msg` - that is ipywidgets' own # internal comm callback, and overriding it breaks all comm handling # (including trait sync). Commands arrive via the `_command` trait. def _on_command(self, change): - self._dispatch(change.get("new") or {}) - - def _dispatch(self, command): - action = command.get("action") or command.get("type") - version = command.get("version") or None - if action == "download": - self._start_download(str(command.get("name", "")), version) - elif action == "cancel": - self._cancel(str(command.get("token", ""))) - elif action == "dismiss": - self._clear_progress(str(command.get("token", ""))) - elif action == "delete": - self._delete(str(command.get("name", "")), version) - elif action == "refresh": - self.refresh() - - def _delete(self, name, version=None): - """Delete a dataset's downloaded file and refresh its status.""" - ds = _catalogue.resolve(name) - if ds is not None: - try: - ds.delete(version=version) - except OSError as error: # read-only dir, permissions, a vanished file - warnings.warn(f"could not delete {name}: {error}", stacklevel=2) - self.refresh() + command = change.get("new") or {} + token = str(command.get("token", "")) + if command.get("action") == "cancel": + self._cancel(token) + elif command.get("action") == "dismiss": + self._clear_progress(token) - # -- downloads ------------------------------------------------------ - def _start_download(self, name, version=None): - """Kick off a background download for ``name`` and show a toast.""" - ds = _catalogue.resolve(name) - if ds is None: - return None - label = _label(name, version) + def begin(self, label): + """Register a new download; return its (monitor, token).""" token = f"{label}-{next(self._counter)}" cancel = threading.Event() with self._lock: self._cancels[token] = cancel + self._labels[token] = label # Show the toast immediately - indeterminate until the first bytes, # and it also covers the cached case where no bytes ever flow. self._set_progress(token, label, 0, 0) - monitor = _WidgetProgress(self, token, label, cancel) - future = _get_executor().submit( - ds.download, progressbar=monitor, background=False, version=version - ) - future.add_done_callback( - lambda f, tk=token, nm=label: self._finish_download(tk, nm, f) - ) - return future + return _WidgetProgress(self, token, label, cancel), token - def _finish_download(self, token, name, future): + def finish(self, token, future): with self._lock: self._cancels.pop(token, None) + label = self._labels.pop(token, token) error = future.exception() if error is not None and not isinstance(error, DownloadCancelled): - self._set_error(token, name, str(error)) + self._set_error(token, label, str(error)) else: self._clear_progress(token) - self.refresh() def _cancel(self, token): with self._lock: @@ -277,15 +233,83 @@ def _clear_progress(self, token): if downloads.pop(token, None) is not None: self.downloads = downloads - def _set_error(self, token, name, message): + def _set_error(self, token, label, message): with self._lock: downloads = dict(self.downloads) - downloads[token] = {"label": name, "error": message} + downloads[token] = {"label": label, "error": message} self.downloads = downloads + return DownloadsWidget + + +@functools.cache +def _make_browser_class(): + """Build the ``DatasetBrowser`` class, importing anywidget lazily.""" + import traitlets + + class DatasetBrowser(_make_downloads_class()): + """Interactive, hoverable list of the emdatabase datasets.""" + + _esm = _STATIC / "browser.js" + + groups = traitlets.List().tag(sync=True) + data_dir = traitlets.Unicode().tag(sync=True) + n_downloaded = traitlets.Int().tag(sync=True) + n_total = traitlets.Int().tag(sync=True) + + def __init__(self, **kwargs): + super().__init__(**kwargs) + self.refresh() + + def refresh(self): + """Rebuild the catalogue - re-checks which files are on disk.""" + cat = _catalogue.catalogue() + self.data_dir = cat["data_dir"] + self.groups = cat["groups"] + self.n_downloaded = cat["n_downloaded"] + self.n_total = cat["n_total"] + + def _on_command(self, change): + command = change.get("new") or {} + action = command.get("action") + name = str(command.get("name", "")) + version = command.get("version") or None + if action == "download": + self._start_download(name, version) + elif action == "delete": + self._delete(name, version) + elif action == "refresh": + self.refresh() + else: + super()._on_command(change) + + def _delete(self, name, version=None): + """Delete a dataset's downloaded file and refresh its status.""" + ds = _catalogue.resolve(name) + if ds is not None: + _delete_file(ds, name, version) + self.refresh() + + def _start_download(self, name, version=None): + """Kick off a background download for ``name`` and show a toast.""" + ds = _catalogue.resolve(name) + if ds is None: + return None + monitor, token = self.begin(_label(name, version)) + future = _get_executor().submit( + ds.download, progressbar=monitor, background=False, version=version + ) + future.add_done_callback(lambda f, tk=token: self.finish(tk, f)) + return future + + def finish(self, token, future): + super().finish(token, future) + self.refresh() + return DatasetBrowser +@functools.cache def _make_card_class(): """Build the ``DatasetCard`` class (one dataset), importing anywidget lazily.""" import anywidget @@ -328,10 +352,7 @@ def _on_command(self, change): elif action == "dismiss": self.download = {} elif action == "delete": - try: - self._dataset.delete(version=version) - except OSError as error: # read-only dir, permissions, a vanished file - warnings.warn(f"could not delete {self._name}: {error}", stacklevel=2) + _delete_file(self._dataset, self._name, version) self.refresh() elif action == "refresh": self.refresh() @@ -368,10 +389,6 @@ def _set_progress(self, token, label, done, total): return DatasetCard -_browser_class = None -_card_class = None - - def card(dataset): """Return an interactive card widget for a single dataset (Jupyter). @@ -379,16 +396,14 @@ def card(dataset): Requires anywidget (``pip install emdatabase[widget]``). """ _prepare_frontend() - global _card_class - if _card_class is None: - try: - _card_class = _make_card_class() - except ImportError as error: - raise ImportError( - "Displaying a dataset needs anywidget. Install it with " - "`pip install emdatabase[widget]` (or `pip install anywidget`)." - ) from error - return _card_class(dataset) + try: + cls = _make_card_class() + except ImportError as error: + raise ImportError( + "Displaying a dataset needs anywidget. Install it with " + "`pip install emdatabase[widget]` (or `pip install anywidget`)." + ) from error + return cls(dataset) def browse(**kwargs): @@ -399,16 +414,14 @@ def browse(**kwargs): metadata on hover, and downloads on click with a live progress toast. """ _prepare_frontend() - global _browser_class - if _browser_class is None: - try: - _browser_class = _make_browser_class() - except ImportError as error: - raise ImportError( - "emdatabase.browse() needs anywidget. Install it with " - "`pip install emdatabase[widget]` (or `pip install anywidget`)." - ) from error - return _browser_class(**kwargs) + try: + cls = _make_browser_class() + except ImportError as error: + raise ImportError( + "emdatabase.browse() needs anywidget. Install it with " + "`pip install emdatabase[widget]` (or `pip install anywidget`)." + ) from error + return cls(**kwargs) # --------------------------------------------------------------------------- @@ -416,83 +429,19 @@ def browse(**kwargs): # --------------------------------------------------------------------------- +@functools.cache def _make_toasts_class(): """Build the singleton ``DownloadToasts`` widget, importing anywidget lazily.""" - import anywidget - import traitlets - class DownloadToasts(anywidget.AnyWidget): + class DownloadToasts(_make_downloads_class()): """An invisible anchor that floats download toasts at the viewport corner.""" _esm = _STATIC / "toasts.js" - _css = _STATIC / "browser.css" - - downloads = traitlets.Dict().tag(sync=True) - _command = traitlets.Dict().tag(sync=True) - - def __init__(self, **kwargs): - super().__init__(**kwargs) - self._lock = threading.RLock() - self._cancels: dict[str, threading.Event] = {} - self._labels: dict[str, str] = {} - self._counter = itertools.count() - self.observe(self._on_command, names="_command") - - def _on_command(self, change): - command = change.get("new") or {} - action = command.get("action") - token = str(command.get("token", "")) - if action == "cancel": - with self._lock: - event = self._cancels.get(token) - if event is not None: - event.set() - elif action == "dismiss": - self._clear_progress(token) - - def begin(self, label): - """Register a new download; return its (monitor, token).""" - token = f"{label}-{next(self._counter)}" - cancel = threading.Event() - with self._lock: - self._cancels[token] = cancel - self._labels[token] = label - self._set_progress(token, label, 0, 0) - return _WidgetProgress(self, token, label, cancel), token - - def finish(self, token, future): - with self._lock: - self._cancels.pop(token, None) - label = self._labels.pop(token, token) - error = future.exception() if future is not None else None - if error is not None and not isinstance(error, DownloadCancelled): - self._set_error(token, label, str(error)) - else: - self._clear_progress(token) - - def _set_progress(self, token, label, done, total): - with self._lock: - downloads = dict(self.downloads) - downloads[token] = {"label": label, "done": int(done), "total": int(total)} - self.downloads = downloads - - def _clear_progress(self, token): - with self._lock: - downloads = dict(self.downloads) - if downloads.pop(token, None) is not None: - self.downloads = downloads - - def _set_error(self, token, label, message): - with self._lock: - downloads = dict(self.downloads) - downloads[token] = {"label": label, "error": message} - self.downloads = downloads return DownloadToasts _toasts = None -_toasts_class = None def _in_notebook(): @@ -521,15 +470,13 @@ def _get_toasts(): the views share one body-level toast root (see toasts.js), so re-anchoring never duplicates the toasts. """ - global _toasts, _toasts_class + global _toasts if not _in_notebook(): return None _prepare_frontend() try: - if _toasts_class is None: - _toasts_class = _make_toasts_class() if _toasts is None: - _toasts = _toasts_class() + _toasts = _make_toasts_class()() from IPython.display import display display(_toasts) From a15695936e0a20b71dec9db06982fc82b3450239 Mon Sep 17 00:00:00 2001 From: arthurmccray Date: Mon, 14 Sep 2026 16:26:30 -0700 Subject: [PATCH 10/18] simplifying metadata --- emdatabase/downloadable_dataset.py | 4 +- emdatabase/metadata.py | 210 ++++++++--------------------- emdatabase/tests/test_metadata.py | 60 +-------- 3 files changed, 66 insertions(+), 208 deletions(-) diff --git a/emdatabase/downloadable_dataset.py b/emdatabase/downloadable_dataset.py index 2e46713..50c123a 100644 --- a/emdatabase/downloadable_dataset.py +++ b/emdatabase/downloadable_dataset.py @@ -423,9 +423,7 @@ def size(self) -> str: def __repr__(self): # __class__ rather than its __name__ nested one set of angle brackets # inside another, which a list of results made unreadable. - techniques = ", ".join(self.metadata.technique) - headline = " · ".join(p for p in (self.file, techniques, self.size) if p) - return f"<{type(self).__name__} {headline}>" + return f"<{type(self).__name__} {self.metadata.headline}>" def _repr_mimebundle_(self, **kwargs): """Rich display in Jupyter: an interactive card with download/metadata. diff --git a/emdatabase/metadata.py b/emdatabase/metadata.py index 7a05402..fe88070 100644 --- a/emdatabase/metadata.py +++ b/emdatabase/metadata.py @@ -4,7 +4,8 @@ ``additionalProperties: false``, so an entry is a fixed set of fields rather than an open bag. :class:`DatasetMetadata` is that set, and :meth:`DatasetMetadata.from_spec` is the only way one is built: it turns a -parsed YAML mapping into the record and refuses anything the schema would. +parsed YAML mapping into the record. What checks an entry is the schema, +through :func:`validate_document`. A ``kind: weights`` entry is a family rather than a single file: it declares a ``latest`` link and a dated version for each state that link has served, both as @@ -42,8 +43,6 @@ # Files in index/ that are not dataset collections. NON_DATASET_FILES = frozenset({VENDORS_PATH.name, TECHNIQUES_PATH.name, TEMPLATE_PATH.name}) -REQUIRED_FIELDS = ("description", "source", "file") - # Wrap width for __str__; narrow enough to stay readable in a notebook cell. _STR_WIDTH = 88 @@ -106,11 +105,6 @@ def acquisition_techniques() -> tuple[str, ...]: return tuple(load_techniques()["acquisition"]) -def ml_tasks() -> tuple[str, ...]: - """What a model does - what a ``kind: weights`` entry declares as well.""" - return tuple(load_techniques()["ml_task"]) - - def techniques() -> tuple[str, ...]: """The whole vocabulary, acquisition first, each in the order of the file.""" vocabulary = load_techniques() @@ -132,26 +126,8 @@ def check_vendor(value: str, known: Iterable[str], cutoff: float = 0.8) -> tuple return _check_known(value, known, "vendors.yaml", "vendor", cutoff) -def check_technique( - value: str, known: Iterable[str] | None = None, cutoff: float = 0.8 -) -> tuple[str, str] | None: - """``(level, message)`` for a technique string, or ``None`` if it is known. - - The same rule as :func:`check_vendor`: a near-miss of a vocabulary entry is - a misspelling and an ``"error"``, and anything else is a ``"warning"`` - asking for it to be added to ``techniques.yaml``. - """ - return _check_known( - value, - techniques() if known is None else known, - "techniques.yaml", - "technique", - cutoff, - ) - - def _check_known( - value: str, known: Iterable[str], source: str, noun: str, cutoff: float + value: str, known: Iterable[str], source: str, noun: str, cutoff: float = 0.8 ) -> tuple[str, str] | None: known = list(known) if not value or value in known: @@ -195,12 +171,7 @@ def validate_document( decides whether a problem is a failed test, a comment on an issue or a non-zero exit. """ - try: - from jsonschema.validators import validator_for - except ImportError as error: # pragma: no cover - depends on the environment - raise ImportError( - "validating a dataset YAML needs jsonschema: pip install emdatabase[dev]" - ) from error + from jsonschema.validators import validator_for if not isinstance(document, Mapping): got = "nothing" if document is None else f"a {type(document).__name__}" @@ -219,9 +190,8 @@ def validate_document( ] vendors = load_vendors() - vocabulary = load_techniques() - known_techniques = list(vocabulary["acquisition"]) + list(vocabulary["ml_task"]) - ml_task_names = set(vocabulary["ml_task"]) + known_techniques = techniques() + ml_task_names = set(load_techniques()["ml_task"]) for name, spec in document.items(): if not isinstance(spec, Mapping): continue @@ -235,27 +205,21 @@ def validate_document( for label in versions if not isinstance(label, str) ] - for name_field, known in ( - ("detector_manufacturer", vendors["detector_manufacturer"]), - ("microscope_vendor", vendors["microscope_vendor"]), - ): - result = check_vendor(spec.get(name_field) or "", known) - if result is None: - continue - level, message = result - line = f"{_where(origin)}: {name}: {name_field}: {message}" - if level == "error": - problems.append(line) - else: - warnings.warn(line, stacklevel=2) - declared = _declared_techniques(spec) - for value in declared: - result = check_technique(value, known_techniques) + # (field, value, the names it should be one of, the file they are in, what one is) + names = [ + (f, spec.get(f) or "", vendors[f], "vendors.yaml", "vendor") + for f in ("detector_manufacturer", "microscope_vendor") + ] + names += [ + ("technique", t, known_techniques, "techniques.yaml", "technique") for t in declared + ] + for name_field, value, known, source, noun in names: + result = _check_known(value, known, source, noun) if result is None: continue level, message = result - line = f"{_where(origin)}: {name}: technique: {message}" + line = f"{_where(origin)}: {name}: {name_field}: {message}" if level == "error": problems.append(line) else: @@ -321,55 +285,20 @@ class Author: affiliation: str orcid: str | None = None - @classmethod - def from_spec( - cls, name: str, spec: Mapping[str, Any], origin: Path | str | None = None - ) -> Author: - allowed = {f.name for f in fields(cls)} - unknown = sorted(set(spec) - allowed) - if unknown: - raise TypeError( - f"{_where(origin)}: author {name!r} has unknown field(s) " - f"{', '.join(repr(k) for k in unknown)}; allowed: {', '.join(sorted(allowed))}" - ) - if "affiliation" not in spec: - raise TypeError(f"{_where(origin)}: author {name!r} is missing 'affiliation'") - return cls(affiliation=str(spec["affiliation"]), orcid=spec.get("orcid")) - @dataclass(frozen=True) class ModelInfo: """The model a ``kind: weights`` entry is a checkpoint for. ``class_`` carries the YAML's ``class`` key, which is a Python keyword and - so cannot be a field name; :meth:`from_spec` is where the two are tied - together. + so cannot be a field name; :meth:`DatasetMetadata.from_spec` is where the two + are tied together. """ class_: str framework: str quantem: str | None = None - @classmethod - def from_spec(cls, spec: Mapping[str, Any], origin: Path | str | None = None) -> ModelInfo: - allowed = ("class", "framework", "quantem") - unknown = sorted(set(spec) - set(allowed)) - if unknown: - raise TypeError( - f"{_where(origin)}: model has unknown field(s) " - f"{', '.join(repr(k) for k in unknown)}; allowed: {', '.join(allowed)}" - ) - missing = [name for name in ("class", "framework") if not spec.get(name)] - if missing: - raise TypeError( - f"{_where(origin)}: model is missing {', '.join(repr(k) for k in missing)}" - ) - return cls( - class_=str(spec["class"]), - framework=str(spec["framework"]), - quantem=spec.get("quantem"), - ) - @dataclass(frozen=True) class WeightsVersion: @@ -385,26 +314,6 @@ class WeightsVersion: checksum: str | None = None size_bytes: int | None = None - @classmethod - def from_spec( - cls, label: str, spec: Mapping[str, Any], origin: Path | str | None = None - ) -> WeightsVersion: - allowed = {f.name for f in fields(cls)} - unknown = sorted(set(spec) - allowed) - if unknown: - raise TypeError( - f"{_where(origin)}: {label} has unknown field(s) " - f"{', '.join(repr(k) for k in unknown)}; allowed: {', '.join(sorted(allowed))}" - ) - if not spec.get("url"): - raise TypeError(f"{_where(origin)}: {label} is missing 'url'") - size_bytes = spec.get("size_bytes") - return cls( - url=str(spec["url"]), - checksum=spec.get("checksum"), - size_bytes=None if size_bytes is None else int(size_bytes), - ) - @dataclass(frozen=True, repr=False) class DatasetMetadata: @@ -447,63 +356,60 @@ def from_spec( ) -> DatasetMetadata: """Build a record from a parsed YAML entry. - ``origin`` is the file the entry came from; it only appears in error - messages, where it is the difference between a useful complaint and a - ``TypeError`` from somewhere inside an import. + Checking the entry is :func:`validate_document`'s job, against the + schema. Building the dataclasses still refuses an unknown or a missing + field, as a ``TypeError`` that starts with ``origin``, the file the entry + came from: the difference between a useful complaint and one from + somewhere inside an import. """ - allowed = {f.name for f in fields(cls)} - unknown = sorted(set(spec) - allowed) - if unknown: - raise TypeError( - f"{_where(origin)}: unknown field(s) {', '.join(repr(k) for k in unknown)}; " - f"allowed: {', '.join(sorted(allowed))}" - ) - missing = [name for name in REQUIRED_FIELDS if spec.get(name) is None] - if missing: - raise TypeError( - f"{_where(origin)}: missing required field(s) " - f"{', '.join(repr(k) for k in missing)}" - ) values = dict(spec) - size_bytes = values.get("size_bytes") - values["size_bytes"] = None if size_bytes is None else int(size_bytes) technique = values.get("technique") or () - values["technique"] = ( - (str(technique),) if isinstance(technique, str) else tuple(str(t) for t in technique) - ) - values["tags"] = tuple(str(t) for t in values.get("tags") or ()) - values["authors"] = { - str(name): Author.from_spec(str(name), entry or {}, origin) - for name, entry in (values.get("authors") or {}).items() - } - values["kind"] = str(values.get("kind") or "dataset") - model = values.get("model") - values["model"] = None if model is None else ModelInfo.from_spec(model, origin) - latest = values.get("latest") - values["latest"] = ( - None if latest is None else WeightsVersion.from_spec("latest", latest, origin) - ) - values["versions"] = { - str(label): WeightsVersion.from_spec(str(label), spec or {}, origin) - for label, spec in (values.get("versions") or {}).items() - } - return cls(**values) + values["technique"] = (technique,) if isinstance(technique, str) else tuple(technique) + values["tags"] = tuple(values.get("tags") or ()) + values["kind"] = values.get("kind") or "dataset" + try: + values["authors"] = {} + for name, entry in (spec.get("authors") or {}).items(): + try: + values["authors"][name] = Author(**(entry or {})) + except TypeError as error: # say which author, of however many + raise TypeError(f"author {name!r}: {error}") from None + model = values.get("model") + if model is not None: # `class` is a keyword, so the field is `class_` + values["model"] = ModelInfo( + **{ + ("class_" if key == "class" else key): value + for key, value in model.items() + } + ) + if values.get("latest") is not None: + values["latest"] = WeightsVersion(**values["latest"]) + values["versions"] = { + str(label): WeightsVersion(**pin) + for label, pin in (values.get("versions") or {}).items() + } + return cls(**values) + except TypeError as error: + raise TypeError(f"{_where(origin)}: {error}") from None @property def size(self) -> str: """:attr:`size_bytes` formatted for display, or ``""`` if unknown.""" return format_size(self.size_bytes) + @property + def headline(self) -> str: + """The file, what it is and how big, in one line: ``d.zspy · 4D-STEM · 12.5 MB``.""" + return " · ".join(p for p in (self.file, ", ".join(self.technique), self.size) if p) + def __repr__(self) -> str: """One line naming the file, what it is and how big: enough to tell two records apart in a list without printing a paragraph of description.""" - headline = " · ".join(p for p in (self.file, ", ".join(self.technique), self.size) if p) - return f"<{type(self).__name__} {headline}>" + return f"<{type(self).__name__} {self.headline}>" def __str__(self) -> str: """The whole record, wrapped, with the empty fields left out.""" - headline = " · ".join(p for p in (self.file, ", ".join(self.technique), self.size) if p) - head = [headline] + head = [self.headline] if self.description: head.append(textwrap.fill(self.description, width=_STR_WIDTH)) diff --git a/emdatabase/tests/test_metadata.py b/emdatabase/tests/test_metadata.py index 3620eef..88cb1d8 100644 --- a/emdatabase/tests/test_metadata.py +++ b/emdatabase/tests/test_metadata.py @@ -23,7 +23,6 @@ load_schema, load_techniques, load_vendors, - ml_tasks, techniques, validate_document, validate_file, @@ -60,13 +59,6 @@ def test_yaml_is_valid(path): assert validate_file(path) == [] -@pytest.mark.parametrize("path", DATASET_FILES, ids=lambda p: p.name) -def test_index_file_declares_entries(path): - """emdatabase.data reads these directly; an empty one is an import error.""" - document = yaml.safe_load(path.read_text(encoding="utf-8")) - assert isinstance(document, dict) and document, f"{path.name} declares no entries" - - def test_entry_names_are_unique_across_files(): """A name in two files would shadow, and be counted twice in the catalogue.""" names = [name for _, name, _ in ENTRIES] @@ -81,32 +73,15 @@ def test_an_entry_name_no_class_can_have_is_rejected(): assert validate_document({"Fine_Name": entry}) == [] -def test_the_stub_lists_exactly_the_classes_the_loader_builds(): - """CI runs `_create_stubs --check`; both sides read metadata.index_entries.""" - import emdatabase.data as data - from emdatabase._create_stubs import build_pyi_stub - - declared = [ - line.removeprefix("class ").partition("(")[0] - for line in build_pyi_stub().splitlines() - if line.startswith("class ") - ] - assert declared == data.__all__ - - def test_template_is_valid(): """The template is not a dataset, so nothing else here looks at it; a placeholder that does not validate is a contributor's first impression.""" assert validate_file(TEMPLATE_PATH) == [] -@pytest.mark.parametrize("name", [name for _, name, _ in ENTRIES]) -def test_entry_builds_a_record(name): - path, _, spec = next(e for e in ENTRIES if e[1] == name) - metadata = DatasetMetadata.from_spec(spec, path) - assert metadata.description and metadata.source and metadata.file - # The schema defaults `kind`, but every entry shipped here writes it out. - assert spec["kind"] in ("dataset", "weights") +def test_every_shipped_entry_writes_out_its_kind(): + """The schema defaults `kind`, but every entry shipped here says which it is.""" + assert all(spec.get("kind") in ("dataset", "weights") for _, _, spec in ENTRIES) def test_schema_and_dataclass_agree(): @@ -156,7 +131,7 @@ def test_missing_required_field_is_an_error(): def test_author_without_affiliation_is_an_error(): - with pytest.raises(TypeError, match="affiliation"): + with pytest.raises(TypeError, match="author 'Jane Doe': .*'affiliation'"): DatasetMetadata.from_spec( {"description": "d", "source": "s", "file": "f", "authors": {"Jane Doe": {}}} ) @@ -220,20 +195,6 @@ def test_format_size(size_bytes, expected): assert format_size(size_bytes) == expected -@pytest.mark.parametrize(("path", "name", "spec"), ENTRIES, ids=[e[1] for e in ENTRIES]) -def test_declared_vendors_are_spelled_correctly(path, name, spec): - """A vendor close to a known one is a typo; one nothing like it is just new.""" - for field, known in ( - ("detector_manufacturer", VENDORS["detector_manufacturer"]), - ("microscope_vendor", VENDORS["microscope_vendor"]), - ): - result = check_vendor(spec.get(field, ""), known) - if result is None: - continue - level, message = result - assert level != "error", f"{path.name}: {name}: {field}: {message}" - - def test_validate_document_names_the_file_and_the_field(): problems = validate_document({"X": {"description": "d", "file": "f"}}, origin="somewhere.yaml") assert len(problems) == 1 @@ -268,13 +229,10 @@ def level(value): def test_the_vocabulary_is_acquisition_then_ml_task_in_file_order(): vocabulary = load_techniques() assert list(vocabulary) == ["acquisition", "ml_task"] - assert acquisition_techniques() == tuple(vocabulary["acquisition"]) - assert ml_tasks() == tuple(vocabulary["ml_task"]) - assert TECHNIQUES == acquisition_techniques() + ml_tasks() - assert TECHNIQUES[:2] == ("4D-STEM", "Cryo") + assert TECHNIQUES == acquisition_techniques() + tuple(vocabulary["ml_task"]) # "Other" ends the acquisition list, and every ML task is prefixed. assert acquisition_techniques()[-1] == "Other" - assert all(task.startswith("ML - ") for task in ml_tasks()) + assert all(task.startswith("ML - ") for task in vocabulary["ml_task"]) def test_validate_document_reports_a_misspelled_technique_and_warns_about_a_new_one(): @@ -343,11 +301,7 @@ def test_repr_is_one_short_identifying_line(): """The generated dataclass repr is ~1000 chars of mostly description, which is useless as the output of a bare `ds.metadata` in a notebook.""" metadata = _record(technique="4D-STEM", size_bytes=12492298) - text = repr(metadata) - assert "\n" not in text - assert len(text) < 100 - assert text == "" - assert "A description." not in text + assert repr(metadata) == "" def test_repr_lists_every_technique(): From 454c138be8d0cc21b4f24dc6c750f60e7eb20f78 Mon Sep 17 00:00:00 2001 From: arthurmccray Date: Mon, 14 Sep 2026 16:33:31 -0700 Subject: [PATCH 11/18] cleanup --- emdatabase/__init__.py | 3 -- emdatabase/catalogue.py | 71 +++++------------------------- emdatabase/tests/test_directory.py | 54 ----------------------- emdatabase/tests/test_load_data.py | 17 ------- emdatabase/tests/test_locations.py | 61 ------------------------- 5 files changed, 12 insertions(+), 194 deletions(-) delete mode 100644 emdatabase/tests/test_directory.py diff --git a/emdatabase/__init__.py b/emdatabase/__init__.py index 05d47d7..4357851 100644 --- a/emdatabase/__init__.py +++ b/emdatabase/__init__.py @@ -1,11 +1,8 @@ -### Example datasets ### from emdatabase import config, data from emdatabase.config import LocationName, add_location, locations, remove_location from emdatabase.downloadable_dataset import DownloadableDataset from emdatabase.query import filter, list_datasets, list_weights, search # noqa: A004 -__all__ = [] - def browse(**kwargs): """ diff --git a/emdatabase/catalogue.py b/emdatabase/catalogue.py index 1b3e9d6..b411305 100644 --- a/emdatabase/catalogue.py +++ b/emdatabase/catalogue.py @@ -10,13 +10,11 @@ from __future__ import annotations -import inspect -import warnings from collections.abc import Iterable from pathlib import Path from emdatabase.downloadable_dataset import DownloadableDataset -from emdatabase.metadata import DatasetMetadata, format_size +from emdatabase.metadata import format_size # Model weights are grouped under one heading of their own rather than by # technique, and it goes after every technique. @@ -24,52 +22,19 @@ def datasets() -> list[tuple[str, DownloadableDataset]]: - """``(name, dataset)`` for every dataset ``emdatabase.data`` exposes. - - Filtered by ``issubclass`` so the base class and incidental imports in the - module namespace stay out; sorted by name for a stable order. - """ + """``(name, dataset)`` for every entry ``emdatabase.data`` exposes, sorted by name.""" import emdatabase.data as data - out: list[tuple[str, DownloadableDataset]] = [] - for name in getattr(data, "__all__", None) or dir(data): - if name.startswith("_"): - continue - obj = getattr(data, name, None) - if ( - not inspect.isclass(obj) - or obj is DownloadableDataset - or not issubclass(obj, DownloadableDataset) - ): - continue - try: - out.append((name, obj())) - except TypeError as error: - # from_spec rejects a malformed entry. emdatabase.data validates at - # import so this should be unreachable, but a dataset silently - # missing from the browser is the wrong way to find out otherwise. - warnings.warn(f"skipping dataset {name!r}: {error}", stacklevel=2) - return sorted(out, key=lambda kv: kv[0].lower()) + return sorted( + ((name, getattr(data, name)()) for name in data.__all__), key=lambda kv: kv[0].lower() + ) def resolve(name: str) -> DownloadableDataset | None: - """The dataset instance for a catalogue name, or ``None``.""" + """The dataset instance for a catalogue name, or ``None`` if there is no such entry.""" import emdatabase.data as data - obj = getattr(data, str(name), None) - if ( - not inspect.isclass(obj) - or obj is DownloadableDataset - or not issubclass(obj, DownloadableDataset) - ): - return None - return obj() - - -def _techniques(md: DatasetMetadata) -> list[str]: - """Every technique the entry declares, or ``["Other"]`` if it declares none.""" - found = [t.strip() for t in md.technique if t.strip()] - return found or ["Other"] + return getattr(data, name)() if name in data.__all__ else None def _join(*parts) -> str: @@ -100,11 +65,7 @@ def _versions(ds: DownloadableDataset) -> list[dict]: rows = [] for version in ds.versions: pin = ds.metadata.versions[version] - try: - found = ds.filepaths(version) - except Exception: - found = [] - path = found[0] if found else None + path = ds.filepath(version) rows.append( { "version": version, @@ -126,10 +87,7 @@ def entry(name: str, ds: DownloadableDataset) -> dict: describe ``latest``; ``versions`` holds the dated snapshots. """ md = ds.metadata - try: - found = ds.filepaths() - except Exception: - found = [] + found = ds.filepaths() path = found[0] if found else None # The copy in the user's own directory, which may sit behind a shared one # in the search order. It is the only copy delete() will touch, so the @@ -138,7 +96,7 @@ def entry(name: str, ds: DownloadableDataset) -> dict: row = { "name": name, "kind": md.kind, - "technique": _techniques(md), + "technique": list(md.technique) or ["Other"], "size": md.size, "downloaded": path is not None, "location": _location(path), @@ -203,12 +161,6 @@ def ordered_groups(names: Iterable[str]) -> list[str]: return sorted(set(names), key=_order) -def _groups(row: dict) -> list[str]: - """The groups a row belongs to - every technique it declares, or, for a - weights family, the one weights heading instead.""" - return [WEIGHTS_GROUP] if row["kind"] == "weights" else list(row["technique"]) - - def catalogue(kind: str | None = None) -> dict: """The whole browser payload, grouped by technique. @@ -233,7 +185,8 @@ def catalogue(kind: str | None = None) -> dict: items = [it for it in items if it["kind"] == kind] by_group: dict[str, list[dict]] = {} for it in items: - for group in _groups(it): + # Every technique a row declares, or for a weights family the one heading. + for group in [WEIGHTS_GROUP] if it["kind"] == "weights" else it["technique"]: by_group.setdefault(group, []).append(it) groups = [{"technique": g, "items": by_group[g]} for g in ordered_groups(by_group)] return { diff --git a/emdatabase/tests/test_directory.py b/emdatabase/tests/test_directory.py deleted file mode 100644 index 46d147b..0000000 --- a/emdatabase/tests/test_directory.py +++ /dev/null @@ -1,54 +0,0 @@ -"""Tests for the personal directory handling. - -These exercise where files land, not what is in them, so they use the smallest -dataset in the index rather than a large one. -""" - -from pathlib import Path - -import pooch - -from emdatabase import config, data -from emdatabase.tests.test_load_data import TINY_DATASET - -DEFAULT_DIR = Path(pooch.os_cache("emdatabase")) - - -def test_data_dir_defaults_to_the_cache(): - assert config.data_dir() == DEFAULT_DIR - - -def test_add_location_personal_sets_the_data_dir(tmp_path): - config.add_location(tmp_path, name="personal", persist=False) - assert config.data_dir() == tmp_path - - -def test_saving_to_configured_dir(tmp_path): - """A dataset downloads into whatever personal directory is configured.""" - config.add_location(tmp_path, name="personal", persist=False) - dataset = getattr(data, TINY_DATASET)() - dest = dataset.download(progressbar=False, background=False) - assert (tmp_path / dataset.file).exists() - # a second download must reuse the file rather than refetch it - assert dataset.download(progressbar=False, background=False) == dest - - -def test_saving_to_explicit_dir(tmp_path): - """An explicit destination overrides the configured personal directory.""" - other = tmp_path / "elsewhere" - configured = tmp_path / "configured" - configured.mkdir() - config.add_location(configured, name="personal", persist=False) - dataset = getattr(data, TINY_DATASET)() - dest = dataset.download(destination=str(other), progressbar=False, background=False) - assert "elsewhere" in str(dest) - assert (other / dataset.file).exists() - - -def test_filepath_reports_missing_and_present(tmp_path): - """filepath() is None until the file is there, then returns the path.""" - config.add_location(tmp_path, name="personal", persist=False) - dataset = getattr(data, TINY_DATASET)() - assert dataset.filepath() is None - dataset.download(progressbar=False, background=False) - assert dataset.filepath() == tmp_path / dataset.file diff --git a/emdatabase/tests/test_load_data.py b/emdatabase/tests/test_load_data.py index b8895b7..6f735af 100644 --- a/emdatabase/tests/test_load_data.py +++ b/emdatabase/tests/test_load_data.py @@ -19,7 +19,6 @@ import pytest import emdatabase.data as data -from emdatabase import catalogue from emdatabase.data import MgONanoCrystals, NiEBSDLarge from emdatabase.downloadable_dataset import ( _PENDING, @@ -387,22 +386,6 @@ def block(): release.set() -def test_a_malformed_entry_warns_instead_of_vanishing(monkeypatch): - """catalogue.datasets() used to swallow every exception, so a bad dataset - just disappeared from the browser with nothing said.""" - broken = type( - "BrokenDataset", - (DownloadableDataset,), - {"_spec": {"description": "d", "source": "s"}, "_metadata": None}, # no 'file' - ) - monkeypatch.setattr(data, "BrokenDataset", broken, raising=False) - monkeypatch.setattr(data, "__all__", [*data.__all__, "BrokenDataset"]) - - with pytest.warns(UserWarning, match="BrokenDataset"): - found = dict(catalogue.datasets()) - assert "BrokenDataset" not in found # skipped, but not silently - - def test_a_caller_s_progress_bar_is_not_replaced_by_the_toast(tmp_path, monkeypatch): """The Jupyter toast stands in for the default bar, not for one passed in.""" import emdatabase.widget as widget_mod diff --git a/emdatabase/tests/test_locations.py b/emdatabase/tests/test_locations.py index e74d630..3e11d3e 100644 --- a/emdatabase/tests/test_locations.py +++ b/emdatabase/tests/test_locations.py @@ -51,18 +51,6 @@ def _configure(tmp_path, **shared): return user -def test_a_shared_location_is_searched_before_the_personal_one(tmp_path): - user = _configure(tmp_path, group="group") - group = tmp_path / "group" - - ds = _dataset() - assert ds.filepath() is None # nowhere yet - (user / ds.file).write_bytes(b"user") - assert ds.filepath() == user / ds.file # found in the personal dir - (group / ds.file).write_bytes(b"group") - assert ds.filepath() == group / ds.file # the shared location wins - - def test_download_uses_a_shared_copy_without_refetching(tmp_path): user = _configure(tmp_path, group="group") group = tmp_path / "group" @@ -74,17 +62,6 @@ def test_download_uses_a_shared_copy_without_refetching(tmp_path): assert not (user / ds.file).exists() # nothing downloaded to the personal dir -def test_search_order_is_the_shared_ones_in_order_then_personal(tmp_path): - user = _configure(tmp_path, first="a", second="b") - assert config.data_search_dirs() == [tmp_path / "a", tmp_path / "b", user] - - -def test_env_var_publishes_a_shared_location(tmp_path, monkeypatch): - monkeypatch.setenv("EMDATABASE_LOCATIONS__GROUP", str(tmp_path / "group")) - config.refresh() - assert config.locations()[0] == config.Location("group", tmp_path / "group", "shared") - - def test_filepaths_reports_every_copy(tmp_path): """A shared copy and your own download of the same file coexist.""" user = _configure(tmp_path, group="group") @@ -122,25 +99,6 @@ def test_entry_reports_the_location_name(tmp_path): assert row["downloaded"] is True # and still resolves -def test_entry_location_is_personal_when_nothing_is_shared(tmp_path): - user = _configure(tmp_path) - - ds = _dataset() - (user / ds.file).write_bytes(b"user") - row = catalogue.entry(TINY_DATASET, ds) - assert row["location"] == "personal" - assert row["user_path"] == str(user / ds.file) - - -def test_downloads_go_to_the_personal_directory(tmp_path): - user = _configure(tmp_path, group="group") - - ds = _dataset() - path = ds.download(progressbar=False, background=False) - assert path == user / ds.file - assert not (tmp_path / "group" / ds.file).exists() - - def test_the_catalogue_payload_carries_the_locations(tmp_path): user = _configure(tmp_path, group="group") payload = catalogue.catalogue() @@ -198,12 +156,6 @@ def test_a_path_is_never_a_location_name(tmp_path): assert config.resolve_destination("group") == tmp_path / "group" assert config.resolve_destination(Path("group")) == Path("group") - assert DownloadableDataset._resolve_destination(Path("group")) == Path("group") - - -def test_resolve_destination_passes_none_through(tmp_path): - _configure(tmp_path, group="group") - assert config.resolve_destination(None) is None def test_delete_by_location_name_removes_the_shared_copy(tmp_path): @@ -222,16 +174,3 @@ def test_delete_by_location_name_removes_the_shared_copy(tmp_path): assert ds.delete(destination="group") is True assert not (group / ds.file).exists() assert ds.filepath() is None # nothing left anywhere - - -def test_delete_personal_by_name_is_the_bare_delete(tmp_path): - user = _configure(tmp_path, group="group") - group = tmp_path / "group" - - ds = _dataset() - (group / ds.file).write_bytes(b"group") - (user / ds.file).write_bytes(b"user") - - assert ds.delete(destination="personal") is True - assert not (user / ds.file).exists() - assert (group / ds.file).exists() From e18ecf946e17f4f0c00e84cbcf5c4a6b74b7a93b Mon Sep 17 00:00:00 2001 From: arthurmccray Date: Mon, 14 Sep 2026 17:15:45 -0700 Subject: [PATCH 12/18] cleanup --- emdatabase/_create_stubs.py | 47 +++---- emdatabase/tests/test_fill_download_fields.py | 16 +-- emdatabase/tests/test_load_data.py | 118 +++++------------- emdatabase/tests/test_query.py | 37 ------ 4 files changed, 52 insertions(+), 166 deletions(-) diff --git a/emdatabase/_create_stubs.py b/emdatabase/_create_stubs.py index 804600b..d5b946c 100644 --- a/emdatabase/_create_stubs.py +++ b/emdatabase/_create_stubs.py @@ -37,38 +37,27 @@ def build_docstring(dataset_dict) -> str: def build_pyi_stub() -> str: """The contents of the ``.pyi`` stub for the current dataset YAML.""" - stub_lines = [ + # The same entries emdatabase.data builds classes from, so the stub cannot + # claim a class the loader skipped or miss one it made. + entries = list(index_entries()) + lines = [ "# Auto-generated stub file for emdatabase", "from emdatabase.downloadable_dataset import DownloadableDataset", "", ] - - dataset_classes = [] - - # The same entries emdatabase.data builds classes from, so the stub cannot - # claim a class the loader skipped or miss one it made. - for entry in index_entries(): - description = build_docstring(entry.spec) - - stub_lines.append(f"class {entry.class_name}(DownloadableDataset):") - stub_lines.append(' """') - stub_lines.append(f" {entry.name}") - if description: - stub_lines.append("") - stub_lines.append(f" {description}") - stub_lines.append(' """') - stub_lines.append(" ...") - stub_lines.append("") - - dataset_classes.append(entry.class_name) - - stub_lines.append(f"__all__ = {dataset_classes}") - return "\n".join(stub_lines) - - -def generate_pyi_stub() -> None: - """Write the stub file.""" - STUB_PATH.write_text(build_pyi_stub(), encoding="utf-8") + for entry in entries: + lines += [ + f"class {entry.class_name}(DownloadableDataset):", + ' """', + f" {entry.name}", + "", + f" {build_docstring(entry.spec)}", + ' """', + " ...", + "", + ] + lines.append(f"__all__ = {[entry.class_name for entry in entries]}") + return "\n".join(lines) if __name__ == "__main__": @@ -79,4 +68,4 @@ def generate_pyi_stub() -> None: "regenerate it with `python -m emdatabase._create_stubs`" ) else: - generate_pyi_stub() + STUB_PATH.write_text(build_pyi_stub(), encoding="utf-8") diff --git a/emdatabase/tests/test_fill_download_fields.py b/emdatabase/tests/test_fill_download_fields.py index b869765..23b41a7 100644 --- a/emdatabase/tests/test_fill_download_fields.py +++ b/emdatabase/tests/test_fill_download_fields.py @@ -97,13 +97,13 @@ def test_a_complete_index_is_not_rewritten(script, index, tmp_path): def test_only_the_files_named_are_looked_at(script, index, tmp_path): - """The workflow passes the files a pull request changed.""" + """The workflow passes the files a pull request changed, a deleted one included.""" _, directory, write = index path = write() other = directory / "Other.yaml" other.write_text(path.read_text(encoding="utf-8").replace("MyData:", "Other:"), "utf-8") - code, _ = _run(script, directory, tmp_path, str(path)) + code, _ = _run(script, directory, tmp_path, str(path), str(directory / "Removed.yaml")) assert code == 0 assert "checksum" in path.read_text(encoding="utf-8") assert "checksum" not in other.read_text(encoding="utf-8") @@ -140,15 +140,3 @@ def test_an_entry_that_will_not_validate_is_not_written(script, index, tmp_path) assert code == 1 assert path.read_text(encoding="utf-8") == before assert "Tomograhy" in summary - - -def test_a_file_the_pull_request_deleted_is_skipped(script, index, tmp_path): - """A removed entry is named by the diff but is not there to read.""" - _, directory, write = index - path = write() - gone = directory / "Removed.yaml" - - code, _ = _run(script, directory, tmp_path, str(path), str(gone)) - assert code == 0 - assert "checksum" in path.read_text(encoding="utf-8") - assert not gone.exists() diff --git a/emdatabase/tests/test_load_data.py b/emdatabase/tests/test_load_data.py index 6f735af..ddb7da2 100644 --- a/emdatabase/tests/test_load_data.py +++ b/emdatabase/tests/test_load_data.py @@ -2,11 +2,12 @@ The expensive part of testing a download index is not the download - it is knowing that every ``source`` still resolves. Checking that costs a HEAD request -per dataset, so it is done for all of them. Actually pulling bytes only proves -that pooch and the checksum verification are wired up correctly, which is -identical for every entry, so it is done once with the smallest file in the -index. The large downloads are marked ``slow`` and deselected by default; run -them with ``pytest -m slow``. +per dataset, so it is done for all of them, under ``-m network``. Actually +pulling bytes only proves that pooch and the checksum verification are wired up +correctly, which is identical for every entry: the default run does that against +``conftest``'s local server (``test_locations``, ``test_weights``), and +``-m slow`` against the real hosts, with the smallest file in the index among +them. """ import os @@ -105,20 +106,12 @@ def test_source_url_resolves(name, version): def test_metadata_is_complete(name): """Entries need enough metadata for pooch to fetch and verify them.""" dataset = getattr(data, name)() - assert dataset.source, f"{name} has no source" - assert dataset.file, f"{name} has no file" - assert dataset.metadata.description, f"{name} has no description" assert dataset.checksum and dataset.checksum.startswith("md5:"), ( f"{name} has no md5 checksum, so a corrupt or truncated download would go unnoticed" ) assert dataset.size_bytes, f"{name} has no size_bytes" -def test_download_url_joins_the_source_and_the_file(): - dataset = getattr(data, TINY_DATASET)() - assert dataset.download_url == f"{dataset.source}/{dataset.file}" - - def test_an_explicit_url_is_what_gets_downloaded(): """A link that does not end in the file name is given whole, as ``url``.""" dataset = DownloadableDataset( @@ -130,6 +123,7 @@ def test_an_explicit_url_is_what_gets_downloaded(): assert dataset.download_url == "https://drive.google.com/uc?export=download&id=abc" +@pytest.mark.slow def test_download_verifies_checksum(tmp_path): """A real download, to prove pooch and checksum verification are wired up.""" dataset = getattr(data, TINY_DATASET)() @@ -141,35 +135,25 @@ def test_download_verifies_checksum(tmp_path): assert path == tmp_path / dataset.file -def test_download_default_returns_path_handle(tmp_path): - """The default download runs in the background and hands back a path handle - that is a real ``Path`` and resolves to the downloaded file.""" - dataset = getattr(data, TINY_DATASET)() - handle = dataset.download(destination=tmp_path, progressbar=False) - assert isinstance(handle, DatasetPath) - assert isinstance(handle, Path) - # Using it as a path blocks until the bytes are there, then behaves normally. - assert os.fspath(handle) == str(tmp_path / dataset.file) - assert handle.is_file() - assert (tmp_path / dataset.file).exists() - assert handle.done - +def _slow_retrieve(dataset, tmp_path, started=None): + """A stand-in for ``_retrieve``: it takes a moment, then writes ``payload``.""" -def test_download_handle_is_nonblocking_then_blocks_on_use(tmp_path, monkeypatch): - """download() returns before the file exists; touching the path waits for it.""" - dataset = getattr(data, TINY_DATASET)() - started = threading.Event() - - def slow_retrieve( - destination=None, progressbar=True, chunk_size=4096, version=None, refresh=False - ): - started.set() + def retrieve(destination=None, progressbar=True, chunk_size=4096, version=None, refresh=False): + if started is not None: + started.set() time.sleep(0.4) target = tmp_path / dataset.file target.write_bytes(b"payload") return str(target) - monkeypatch.setattr(dataset, "_retrieve", slow_retrieve) + return retrieve + + +def test_download_handle_is_nonblocking_then_blocks_on_use(tmp_path, monkeypatch): + """download() returns before the file exists; touching the path waits for it.""" + dataset = getattr(data, TINY_DATASET)() + started = threading.Event() + monkeypatch.setattr(dataset, "_retrieve", _slow_retrieve(dataset, tmp_path, started)) handle = dataset.download(destination=tmp_path, progressbar=False) assert started.wait(2) # the worker thread really started @@ -184,17 +168,7 @@ def test_download_handle_derived_paths_also_wait(tmp_path, monkeypatch): """A path rebuilt from the handle names the same file, so it must wait too.""" dataset = getattr(data, TINY_DATASET)() started = threading.Event() - - def slow_retrieve( - destination=None, progressbar=True, chunk_size=4096, version=None, refresh=False - ): - started.set() - time.sleep(0.4) - target = tmp_path / dataset.file - target.write_bytes(b"payload") - return str(target) - - monkeypatch.setattr(dataset, "_retrieve", slow_retrieve) + monkeypatch.setattr(dataset, "_retrieve", _slow_retrieve(dataset, tmp_path, started)) handle = dataset.download(destination=tmp_path, progressbar=False) assert started.wait(2) @@ -208,22 +182,15 @@ def test_a_path_that_is_not_downloading_never_waits(tmp_path, monkeypatch): """Only the file being fetched is pending - its directory is not.""" dataset = getattr(data, TINY_DATASET)() - def slow_retrieve( - destination=None, progressbar=True, chunk_size=4096, version=None, refresh=False - ): - time.sleep(0.3) - target = tmp_path / dataset.file - target.write_bytes(b"payload") - return str(target) - - monkeypatch.setattr(dataset, "_retrieve", slow_retrieve) + monkeypatch.setattr(dataset, "_retrieve", _slow_retrieve(dataset, tmp_path)) handle = dataset.download(destination=tmp_path, progressbar=False) assert handle.parent.done is True handle.wait() -def test_finished_downloads_leave_no_pending_entry(tmp_path): +def test_finished_downloads_leave_no_pending_entry(tmp_path, monkeypatch): dataset = getattr(data, TINY_DATASET)() + monkeypatch.setattr(dataset, "_retrieve", _slow_retrieve(dataset, tmp_path)) handle = dataset.download(destination=tmp_path, progressbar=False) handle.wait() key = _pending_key(handle) @@ -235,15 +202,6 @@ def test_finished_downloads_leave_no_pending_entry(tmp_path): assert key not in _PENDING -def test_generated_class_can_be_subclassed(): - base = getattr(data, TINY_DATASET) - - class Subclass(base): - pass - - assert Subclass().file == base().file - - def test_keyword_overrides_leave_the_class_spec_alone(): base = getattr(data, TINY_DATASET) overridden = base(checksum="md5:" + "0" * 32) @@ -264,29 +222,17 @@ def test_a_dataset_without_a_source_is_an_error(): DownloadableDataset() -def test_download_handle_propagates_errors(tmp_path): +def test_download_handle_propagates_errors(tmp_path, monkeypatch): """A failed background download raises when the handle is consumed.""" - dataset = getattr(data, TINY_DATASET)(checksum="md5:" + "0" * 32) - handle = dataset.download(destination=tmp_path, progressbar=False) - with pytest.raises(Exception): - os.fspath(handle) + dataset = getattr(data, TINY_DATASET)() + def fail(*args): + raise ValueError("MD5 hash of downloaded file does not match") -def test_download_is_cached(tmp_path): - """A second download of the same file must not refetch it.""" - dataset = getattr(data, TINY_DATASET)() - first = dataset.download(destination=tmp_path, progressbar=False, background=False) - mtime = (tmp_path / dataset.file).stat().st_mtime_ns - second = dataset.download(destination=tmp_path, progressbar=False, background=False) - assert first == second - assert (tmp_path / dataset.file).stat().st_mtime_ns == mtime - - -def test_download_rejects_a_bad_checksum(tmp_path): - """A wrong checksum must raise rather than hand back the file.""" - dataset = getattr(data, TINY_DATASET)(checksum="md5:" + "0" * 32) - with pytest.raises(Exception): - dataset.download(destination=tmp_path, progressbar=False, background=False) + monkeypatch.setattr(dataset, "_retrieve", fail) + handle = dataset.download(destination=tmp_path, progressbar=False) + with pytest.raises(ValueError, match="does not match"): + os.fspath(handle) @pytest.mark.slow diff --git a/emdatabase/tests/test_query.py b/emdatabase/tests/test_query.py index 261dd89..8cd1434 100644 --- a/emdatabase/tests/test_query.py +++ b/emdatabase/tests/test_query.py @@ -27,28 +27,10 @@ def test_datasets_returns_objects_not_rows(): assert names(found) == sorted(n for n, _ in catalogue.datasets()) -def test_search_matches_every_term_across_fields(): - """The widget's rule: all terms must appear, but not in one field.""" - found = names(emdatabase.search("jeol eels")) - assert found - for name in found: - dataset = catalogue.resolve(name) - assert dataset is not None - blob = catalogue.entry(name, dataset)["search"] - assert "jeol" in blob and "eels" in blob - - def test_search_is_case_insensitive(): assert names(emdatabase.search("AMORPHOUS")) == names(emdatabase.search("amorphous")) -def test_search_finds_a_dataset_by_author(): - """Proof the blob reaches past the name - authors are not in the class name.""" - ds = BilayerWS2() - author = next(iter(ds.metadata.authors)) - assert type(ds).__name__ in names(emdatabase.search(author)) - - def test_search_uses_the_same_blob_and_rule_as_the_widget(): query = "direct electron" terms = query.lower().split() @@ -101,19 +83,6 @@ def test_filter_on_technique_tests_membership(two_techniques): assert "In-situ" in ds.metadata.technique -def test_filter_on_a_list_of_techniques_is_any_of(two_techniques): - stem = set(names(emdatabase.filter(technique="4D-STEM"))) - eels = set(names(emdatabase.filter(technique="EELS"))) - either = set(names(emdatabase.filter(technique=["4D-STEM", "EELS"]))) - assert either == stem | eels - assert stem and eels - - -def test_filter_on_tags_tests_membership(): - for ds in emdatabase.filter(tags="Strain"): - assert "Strain" in ds.metadata.tags - - def test_filter_on_authors_tests_membership(): ds = BilayerWS2() author = next(iter(ds.metadata.authors)) @@ -163,12 +132,6 @@ def test_filter_on_downloaded_and_location(tmp_path): assert emdatabase.filter(location="personal") == [] -def test_the_query_api_is_on_the_top_level_namespace(): - for name in ("list_datasets", "search", "filter"): - assert name in emdatabase.__all__ - assert callable(getattr(emdatabase, name)) - - def test_public_names_survive_a_submodule_walk(): """A submodule and a top-level function cannot share a name. From 66279a297663b64b7dc81aab86fdb11e68eec007 Mon Sep 17 00:00:00 2001 From: arthurmccray Date: Tue, 15 Sep 2026 11:46:51 -0700 Subject: [PATCH 13/18] fixing download pbars --- emdatabase/__init__.py | 2 +- emdatabase/downloadable_dataset.py | 39 ++- emdatabase/static/browser.css | 49 ++- emdatabase/static/browser.js | 519 ++++------------------------- emdatabase/static/card.js | 258 +------------- emdatabase/static/common.js | 280 ++++++++++++++++ emdatabase/static/toasts.js | 144 -------- emdatabase/tests/test_load_data.py | 25 +- emdatabase/tests/test_widget.py | 47 ++- emdatabase/widget.py | 225 ++++--------- 10 files changed, 499 insertions(+), 1089 deletions(-) create mode 100644 emdatabase/static/common.js delete mode 100644 emdatabase/static/toasts.js diff --git a/emdatabase/__init__.py b/emdatabase/__init__.py index 4357851..2e61e72 100644 --- a/emdatabase/__init__.py +++ b/emdatabase/__init__.py @@ -10,7 +10,7 @@ def browse(**kwargs): Returns an `anywidget` widget listing every dataset grouped by technique, showing which are downloaded, revealing full metadata on hover, and - downloading on click with a live progress toast. Requires the optional + downloading on click with a live progress bar. Requires the optional `anywidget` dependency (`pip install emdatabase[widget]`). ``display(emdatabase)`` renders the same browser. diff --git a/emdatabase/downloadable_dataset.py b/emdatabase/downloadable_dataset.py index 50c123a..b15ef91 100644 --- a/emdatabase/downloadable_dataset.py +++ b/emdatabase/downloadable_dataset.py @@ -151,7 +151,7 @@ def _shutdown_executor() -> None: A transfer already streaming still runs to completion - there is no way to interrupt pooch mid-read from here - so this shortens the wait rather than - removing it. The widget's toast has a cancel button for that case. + removing it. The browser and a dataset's card have a cancel button for that case. """ global _executor executor, _executor = _executor, None @@ -257,6 +257,10 @@ class _TqdmProgress: up front would flash an empty one on every cached call; pooch assigns ``total`` exactly once, before streaming, which is the moment there is something worth showing. + + A background download calls :meth:`open` up front instead, on the calling + thread. A pool thread does not carry the running cell, so a notebook bar + built there is shown in whichever cell ran last, or not at all. """ def __init__(self, desc: str = "") -> None: @@ -270,11 +274,18 @@ def total(self) -> int: @total.setter def total(self, value: int) -> None: + self._total = int(value or 0) + if self._bar is None: + self.open() + else: + self._bar.reset(total=self._total or None) + + def open(self) -> "_TqdmProgress": + """Show the bar now, before pooch has said how big the file is.""" from tqdm.auto import tqdm - self._total = int(value or 0) self._bar = tqdm( - total=self._total, + total=self._total or None, desc=self._desc, unit="B", unit_scale=True, @@ -282,6 +293,7 @@ def total(self, value: int) -> None: ascii=sys.platform == "win32", leave=True, ) + return self def update(self, n: int) -> None: self._bar.update(n) @@ -543,20 +555,17 @@ def download( # destination or a refresh asks for a fresh one. existing = None if destination is not None or refresh else self.filepath(version) target = existing or self._resolve_destination(destination) / self.filename(version) - # In Jupyter (with the widget installed) a background download pops a - # cancelable toast; the toast's monitor replaces the plain progress bar. - monitor = finish = None - if progressbar is True: # a caller's own Progress is theirs to drive - from emdatabase.widget import _attach_toast - - label = type(self).__name__ + (f"@{version}" if version else "") - monitor, finish = _attach_toast(label) - progress = monitor if monitor is not None else progressbar + # A file that is really coming gets its bar now, from this thread (see + # _TqdmProgress); a caller's own Progress is theirs to drive. + if progressbar is True and (refresh or not target.exists()): + from emdatabase.widget import _in_notebook, _prepare_frontend + + if _in_notebook(): + _prepare_frontend() # quiets pooch's log lines, which repeat the bar in red + progressbar = _TqdmProgress(target.name).open() future = _get_executor().submit( - self._retrieve, destination, progress, chunk_size, version, refresh + self._retrieve, destination, progressbar, chunk_size, version, refresh ) - if finish is not None: - future.add_done_callback(finish) return DatasetPath(target)._attach(future) def _retrieve( diff --git a/emdatabase/static/browser.css b/emdatabase/static/browser.css index 4568691..d7eb582 100644 --- a/emdatabase/static/browser.css +++ b/emdatabase/static/browser.css @@ -1,10 +1,8 @@ -/* browser.css - emdatabase.browse() +/* browser.css - emdatabase.browse() and a dataset's card * * Themed after SpyDE (Catppuccin Mocha), dark by design like the app itself. - * The palette lives on both the widget root and the (body-level) toast root so - * the pinned toasts are themed even though they render outside .emdb. */ -.emdb, .emdb-toast-root { +.emdb { --emdb-base: #1e1e2e; --emdb-mantle: #181825; --emdb-crust: #11111b; @@ -21,9 +19,6 @@ --emdb-red: #f38ba8; --emdb-font: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, sans-serif; --emdb-mono: ui-monospace, "SF Mono", "JetBrains Mono", Menlo, monospace; -} - -.emdb { font-family: var(--emdb-font); font-size: 13px; color: var(--emdb-text); @@ -190,41 +185,37 @@ .emdb-copy-btn:hover { border-color: var(--emdb-blue); background: rgba(137, 180, 250, 0.1); } .emdb-copy-btn.copied { color: var(--emdb-green); border-color: var(--emdb-green); } +/* The browser's details panel stacks the description and load block above + the metadata. */ +.emdb-details .emdb-d-meta { margin-top: 12px; } + /* Single-dataset card (display(dataset)) --------------------------- */ -.emdb-card-host { max-width: 75%; } .emdb-card { padding: 18px 20px; } /* Two columns: a wide description (+ load) on the left, metadata on the right, so the description gets the space and nothing sprawls edge to edge. */ -.emdb-card-cols { +.emdb-card .emdb-d-cols { display: grid; grid-template-columns: minmax(0, 1fr) 220px; gap: 28px; align-items: start; margin-top: 4px; } -.emdb-card-col-main { min-width: 0; } -.emdb-card .emdb-d-desc { max-width: none; } /* fill the main column */ -.emdb-card-col-side .emdb-d-meta { grid-template-columns: 1fr; } -.emdb-inline-toast { - margin: 2px 0 12px; display: flex; flex-direction: column; gap: 6px; - background: var(--emdb-mantle); border: 1px solid var(--emdb-surface0); - border-radius: 8px; padding: 8px 10px; -} -.emdb-inline-toast.cancelling { opacity: 0.65; } -.emdb-inline-toast.error { border-color: var(--emdb-red); } +.emdb-card .emdb-d-main { min-width: 0; } +.emdb-card .emdb-d-meta { grid-template-columns: 1fr; } +/* A card's downloads sit under its status line. */ +.emdb-card .emdb-toasts:not(:empty) { margin: 2px 0 12px; } -/* Toasts: pinned to the bottom-right of the viewport ----------------- */ -.emdb-toast-root { - position: fixed; right: 16px; bottom: 16px; z-index: 99999; - display: flex; flex-direction: column; gap: 10px; width: 300px; - font-family: var(--emdb-font); +/* Download progress ------------------------------------------------- */ +.emdb-toasts { display: flex; flex-direction: column; gap: 8px; } +/* Along the bottom of the browser, side by side in as many columns as fit. */ +.emdb > .emdb-toasts:not(:empty) { + display: grid; grid-template-columns: repeat(auto-fill, minmax(260px, 1fr)); + padding: 10px 14px; border-top: 1px solid var(--emdb-surface0); } .emdb-toast { - background: var(--emdb-base); border: 1px solid var(--emdb-surface0); - border-radius: 9px; padding: 10px 12px; color: var(--emdb-text); - box-shadow: 0 12px 32px rgba(0, 0, 0, 0.55); - display: flex; flex-direction: column; gap: 7px; + background: var(--emdb-mantle); border: 1px solid var(--emdb-surface0); + border-radius: 8px; padding: 8px 10px; color: var(--emdb-text); + display: flex; flex-direction: column; gap: 6px; } .emdb-toast.error { border-color: var(--emdb-red); } .emdb-toast.cancelling { opacity: 0.65; } -.emdb-x:disabled { opacity: 0.5; cursor: default; } .emdb-toast-row { display: flex; align-items: center; justify-content: space-between; gap: 8px; } .emdb-toast-title { font-size: 12.5px; font-weight: 600; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; } diff --git a/emdatabase/static/browser.js b/emdatabase/static/browser.js index 7943cc0..48adda3 100644 --- a/emdatabase/static/browser.js +++ b/emdatabase/static/browser.js @@ -1,527 +1,136 @@ -// browser.js - frontend for emdatabase.browse() +// browser.js - frontend for emdatabase.browse(); widget.py puts common.js first. // // A dataset browser themed after SpyDE (Catppuccin Mocha): technique tabs + // search at the top, a scrollable list on the left with ○/● download status, a -// details panel on the right, and download toasts pinned to the bottom-right of -// the viewport. Rendering is a pure function of state (model traits + a little +// details panel on the right, and each running download's progress along the +// bottom. Rendering is a pure function of state (model traits + a little // local UI state), so a button never gets stuck on a one-shot label. -const MB = 1e6; - -function fmtMB(bytes) { - const mb = bytes / MB; - if (mb >= 100) return mb.toFixed(0); - if (mb >= 10) return mb.toFixed(1); - return mb.toFixed(2); -} - -function esc(value) { - return String(value).replace(/[&<>"']/g, (c) => ( - { "&": "&", "<": "<", ">": ">", '"': """, "'": "'" }[c] - )); -} - -function el(tag, className, html) { - const node = document.createElement(tag); - if (className) node.className = className; - if (html != null) node.innerHTML = html; - return node; -} - -// CamelCase class name -> snake_case variable, e.g. AlNanocrystals -> al_nanocrystals. -// Copy text to the clipboard (clipboard API, with a textarea fallback for -// non-secure contexts) and briefly flash the button so the click is felt. -function copyText(text, btn) { - const done = () => { - const old = btn.textContent; - btn.textContent = "Copied!"; - btn.classList.add("copied"); - setTimeout(() => { btn.textContent = old; btn.classList.remove("copied"); }, 1100); - }; - if (navigator.clipboard && navigator.clipboard.writeText) { - navigator.clipboard.writeText(text).then(done).catch(() => fallbackCopy(text, done)); - } else { - fallbackCopy(text, done); - } -} - -function fallbackCopy(text, done) { - const ta = document.createElement("textarea"); - ta.value = text; - ta.style.position = "fixed"; - ta.style.opacity = "0"; - document.body.appendChild(ta); - ta.select(); - try { document.execCommand("copy"); done(); } catch (e) { /* ignore */ } - ta.remove(); -} - function render({ model, el: root }) { root.classList.add("emdb"); - root.innerHTML = ""; - // --- local UI state -------------------------------------------------- const state = { tab: "All", search: "", selected: null, // name shown in the details panel (sticky) hovered: null, // name under the cursor (transient preview) - version: {}, // name -> chosen version of a weights family ("" = latest) - optimistic: new Set(), // labels just clicked, before Python confirms - cancelling: new Set(), // tokens whose ✕ was clicked, awaiting teardown - activeSig: "", // signature of the active-download set, to avoid churn }; + const view = newView(model, () => drawList()); - // --- static structure ------------------------------------------------ const header = el("div", "emdb-header"); + const top = el("div", "emdb-header-top", + `
◆ Datasets
`); + const count = el("div", "emdb-count"); + const search = el("input", "emdb-search"); + search.type = "text"; + search.placeholder = "Search datasets…"; + search.addEventListener("input", () => { state.search = search.value; drawList(); }); + top.appendChild(count); + header.append(top, search); const tabsEl = el("div", "emdb-tabs"); - const body = el("div", "emdb-body"); const listEl = el("div", "emdb-list"); const detailsEl = el("div", "emdb-details"); - body.appendChild(listEl); - body.appendChild(detailsEl); - root.appendChild(header); - root.appendChild(tabsEl); - root.appendChild(body); - - // Toasts live on so `position: fixed` is relative to the viewport - // (an ancestor transform in the notebook would otherwise trap them). - const toastRoot = el("div", "emdb-toast-root"); - document.body.appendChild(toastRoot); - - // Send a command to Python via a synced trait (reliable two-way sync); the - // nonce makes a repeated action (e.g. two cancels) still register as a change. - let nonce = 0; - function cmd(action, extra) { - model.set("_command", Object.assign({ action, nonce: nonce++ }, extra || {})); - model.save_changes(); - } - - // --- derived data ---------------------------------------------------- - const allItems = () => (model.get("groups") || []).flatMap((g) => g.items); - - function techniques() { - return (model.get("groups") || []).map((g) => g.technique); - } - - // A download is labelled "Name" or "Name@260902"; the list marks the entry - // whatever version is running, the details panel marks the one selected. - function labelFor(name, version) { - return version ? name + "@" + version : name; - } - - function activeLabels() { - const labels = new Set(state.optimistic); - const downloads = model.get("downloads") || {}; - for (const dl of Object.values(downloads)) { - if (!dl.error) labels.add(dl.label); - } - return labels; - } - - function activeNames() { - return new Set([...activeLabels()].map((label) => label.split("@")[0])); - } - - // Which version the details panel is showing: "" is latest, and is all a - // dataset (or a family whose selection has gone away) ever has. - function currentVersion(item) { - const chosen = state.version[item.name]; - if (!chosen) return ""; - return (item.versions || []).some((v) => v.version === chosen) ? chosen : ""; - } - - // The download state of one version: the same shape as an item's own fields, - // which describe latest, so one set of helpers reads either. - function versionState(item, version) { - if (!version) return item; - return (item.versions || []).find((v) => v.version === version) || {}; - } - - function matchesSearch(item) { - if (!state.search) return true; - // `item.search` is a lowercased blob of every field (name, description, - // detector, microscope, tags, authors + affiliations, license, …), so a - // query like "Carter Francis" matches on author, not just the name. - // emdatabase.search() matches this same blob by this same rule; matching - // stays here rather than in the kernel so typing never waits on a round trip. - const blob = item.search || item.name.toLowerCase(); - return state.search.toLowerCase().split(/\s+/).every((term) => blob.includes(term)); - } - - function findItem(name) { - return allItems().find((it) => it.name === name) || null; - } + const body = el("div", "emdb-body"); + body.append(listEl, detailsEl); + const toasts = el("div", "emdb-toasts"); + root.append(header, tabsEl, body, toasts); - // --- header ---------------------------------------------------------- - function drawHeader() { - const nDown = model.get("n_downloaded") || 0; - const nTot = model.get("n_total") || 0; - header.innerHTML = ""; - const left = el("div", "emdb-brand", `◆ Datasets`); - const count = el("div", "emdb-count", `${nDown} / ${nTot} downloaded`); - const top = el("div", "emdb-header-top"); - top.appendChild(left); - top.appendChild(count); - header.appendChild(top); + const allItems = () => model.get("groups").flatMap((g) => g.items); - const search = el("input", "emdb-search"); - search.type = "text"; - search.placeholder = "Search datasets…"; - search.value = state.search; - search.addEventListener("input", () => { - state.search = search.value; - drawList(); - }); - header.appendChild(search); + function drawCount() { + count.textContent = `${model.get("n_downloaded")} / ${model.get("n_total")} downloaded`; } - // --- tabs ------------------------------------------------------------ function drawTabs() { tabsEl.innerHTML = ""; - const tabs = ["All", ...techniques()]; - for (const tab of tabs) { + for (const tab of ["All", ...model.get("groups").map((g) => g.technique)]) { const btn = el("button", "emdb-tab" + (state.tab === tab ? " active" : ""), esc(tab)); - btn.addEventListener("click", () => { - state.tab = tab; - drawTabs(); - drawList(); - }); + btn.addEventListener("click", () => { state.tab = tab; drawTabs(); drawList(); }); tabsEl.appendChild(btn); } } - // --- list ------------------------------------------------------------ + // `item.search` is a lowercased blob of every field (name, description, + // detector, microscope, tags, authors + affiliations, license, …), so a + // query like "Carter Francis" matches on author, not just the name. + // emdatabase.search() matches this same blob by this same rule; matching + // stays here rather than in the kernel so typing never waits on a round trip. + function matchesSearch(item) { + return state.search.toLowerCase().split(/\s+/).every((term) => item.search.includes(term)); + } + function drawList() { - const active = activeNames(); + // A row is marked whichever version of it is running. + const active = new Set([...activeLabels(view)].map((label) => label.split("@")[0])); listEl.innerHTML = ""; - const groups = model.get("groups") || []; - let shown = 0; // A dataset with several techniques is in several groups, so the All view // lists it under the first one and skips it after that. const drawn = new Set(); - for (const group of groups) { + for (const group of model.get("groups")) { if (state.tab !== "All" && group.technique !== state.tab) continue; - const items = group.items.filter(matchesSearch).filter((it) => !drawn.has(it.name)); + const items = group.items.filter((it) => matchesSearch(it) && !drawn.has(it.name)); if (!items.length) continue; if (state.tab === "All") { listEl.appendChild(el("div", "emdb-group-head", esc(group.technique))); + for (const item of items) drawn.add(item.name); } - for (const item of items) { - if (state.tab === "All") drawn.add(item.name); - listEl.appendChild(drawRow(item, active)); - shown += 1; - } - } - if (!shown) { - listEl.appendChild(el("div", "emdb-empty", "No datasets match.")); - } - if (!state.selected && groups.length) { - state.selected = allItems()[0]?.name || null; + for (const item of items) listEl.appendChild(drawRow(item, active.has(item.name))); } + if (!listEl.children.length) listEl.appendChild(el("div", "emdb-empty", "No datasets match.")); + state.selected ??= allItems()[0]?.name; drawDetails(); } function drawRow(item, active) { - const isActive = active.has(item.name); const row = el("div", "emdb-row" + (state.selected === item.name ? " selected" : "")); - const meta = [item.size, item.shape].filter(Boolean).join(" · "); - const glyph = el("span", "emdb-glyph " + glyphClass(item), item.downloaded ? "●" : "○"); - if (inShared(item)) glyph.title = sharedTitle(item); - row.appendChild(glyph); - row.appendChild(el("span", "emdb-name", esc(item.name))); - row.appendChild(el("span", "emdb-meta", esc(meta))); - row.appendChild(drawAction(item, isActive)); - - row.addEventListener("mouseenter", () => { state.hovered = item.name; drawDetails(); }); - row.addEventListener("click", () => { state.selected = item.name; drawList(); }); - return row; - } - - // `location` is the name of the location a copy was found in; "personal" is - // the user's own directory. `where` is an item (which describes latest) or - // one of its versions. - function inShared(where) { - return where.downloaded && where.location && where.location !== "personal"; - } - - // A shared copy and your own can both exist; the tooltip names each. - function sharedTitle(where) { - const lines = ["from " + where.location + ": " + where.path]; - if (where.user_path) lines.push("your copy: " + where.user_path); - return lines.join("\n"); - } - - function glyphClass(item) { - if (!item.downloaded) return "off"; - return inShared(item) ? "shared" : "on"; - } - - function drawAction(item, isActive) { - const wrap = el("span", "emdb-actions"); + const shared = inShared(item); + const glyph = el("span", "emdb-glyph " + (shared ? "shared" : item.downloaded ? "on" : "off"), + item.downloaded ? "●" : "○"); + if (shared) glyph.title = sharedTitle(item); + const actions = el("span", "emdb-actions"); if (item.downloaded) { - wrap.appendChild(el("span", "emdb-check", "✓")); - } else if (isActive) { - wrap.appendChild(el("span", "emdb-spinner", "downloading…")); + actions.appendChild(el("span", "emdb-check", "✓")); + } else if (active) { + actions.appendChild(el("span", "emdb-spinner", "downloading…")); } else { const btn = el("button", "emdb-dl", "Download"); btn.addEventListener("click", (event) => { event.stopPropagation(); - startDownload(item.name); + startDownload(view, item.name, ""); }); - wrap.appendChild(btn); + actions.appendChild(btn); } - return wrap; + row.append(glyph, el("span", "emdb-name", esc(item.name)), + el("span", "emdb-meta", esc(item.size)), actions); + row.addEventListener("mouseenter", () => { state.hovered = item.name; drawDetails(); }); + row.addEventListener("click", () => { state.selected = item.name; drawList(); }); + return row; } - // --- details panel --------------------------------------------------- function drawDetails() { const name = state.hovered || state.selected; - const item = name ? findItem(name) : null; - detailsEl.innerHTML = ""; - if (!item) { - detailsEl.appendChild(el("div", "emdb-details-empty", "Hover or select a dataset.")); - return; - } - const version = currentVersion(item); - const where = versionState(item, version); - const active = activeLabels().has(labelFor(item.name, version)); - - const head = el("div", "emdb-d-head"); - const title = el("div", "emdb-d-title", esc(item.name)); - if (item.kind === "weights") title.appendChild(el("span", "emdb-kind", "weights")); - if ((item.versions || []).length) title.appendChild(versionSelect(item, version)); - head.appendChild(title); - const sub = [(item.technique || []).join(", "), item.size, item.shape] - .filter(Boolean).join(" · "); - head.appendChild(el("div", "emdb-d-sub", esc(sub))); - detailsEl.appendChild(head); - - // status / action line, for whichever version is selected - const statusRow = el("div", "emdb-d-status"); - if (inShared(where)) { - const label = "● " + where.location + (where.user_path ? " + yours" : ""); - const badge = el("span", "emdb-d-badge shared", esc(label)); - badge.title = sharedTitle(where); - statusRow.appendChild(badge); - if (where.user_path) { - const del = el("button", "emdb-delete", "Delete yours"); - del.title = "Remove your copy (" + where.user_path + "). The copy in " - + where.location + " is untouched."; - del.addEventListener("click", () => cmd("delete", { name: item.name, version })); - statusRow.appendChild(del); - } - } else if (where.downloaded) { - statusRow.appendChild(el("span", "emdb-d-badge on", "● downloaded")); - const del = el("button", "emdb-delete", "Delete"); - del.title = "Remove the downloaded file from disk"; - del.addEventListener("click", () => cmd("delete", { name: item.name, version })); - statusRow.appendChild(del); - } else if (active) { - statusRow.appendChild(el("span", "emdb-d-badge", "downloading…")); - } else { - const btn = el("button", "emdb-dl", "Download"); - btn.addEventListener("click", () => startDownload(item.name, version)); - statusRow.appendChild(btn); - } - detailsEl.appendChild(statusRow); - - if (item.description) { - detailsEl.appendChild(el("p", "emdb-d-desc", esc(item.description))); - } - const pairs = [ - ["Detector", item.detector], - ["Microscope", item.microscope], - ["Voltage", item.voltage], - ["Tags", (item.tags || []).join(", ")], - ["Authors", (item.authors || []).join(", ")], - ["License", item.license], - ["File", item.file], - ["DOI", item.doi], - ["Versions", (item.versions || []).map((v) => v.version).join(", ")], - ["Model", item.model_class], - ["Framework", item.model_framework], - ["quantem", item.model_quantem], - ]; - const meta = el("div", "emdb-d-meta"); - for (const [key, value] of pairs) { - if (!value) continue; - const kv = el("div", "emdb-kv"); - kv.appendChild(el("span", "emdb-k", key)); - kv.appendChild(el("span", "emdb-v", esc(value))); - meta.appendChild(kv); - } - detailsEl.appendChild(meta); - - // Load block: copy a ready-to-paste snippet, and the on-disk path. - detailsEl.appendChild(el("div", "emdb-load-label", "Load")); - const snippet = `path = emdatabase.data.${item.name}().download(` - + (version ? `version="${version}"` : "") + ")"; - detailsEl.appendChild(copyRow(snippet, snippet)); - if (where.downloaded && where.path) { - detailsEl.appendChild(copyRow(where.path, where.path, "path")); - } + const item = allItems().find((it) => it.name === name); + if (item) drawEntry(detailsEl, item, view); + else detailsEl.innerHTML = `
Hover or select a dataset.
`; } - // The version picker for a weights family: `latest`, then the dated - // snapshots, ● on the ones already on disk. Everything below it - the - // Download/Delete button, the path and the load snippet - follows it. - function versionSelect(item, version) { - const select = el("select", "emdb-version"); - select.title = "Which version to download, delete or load"; - const choices = [["", "latest"]].concat( - (item.versions || []).map((v) => [v.version, v.version]) - ); - for (const [value, text] of choices) { - const option = document.createElement("option"); - option.value = value; - option.textContent = text + (versionState(item, value).downloaded ? " ●" : ""); - option.selected = value === version; - select.appendChild(option); - } - select.addEventListener("click", (event) => event.stopPropagation()); - select.addEventListener("change", () => { - state.version[item.name] = select.value; - drawDetails(); - }); - return select; - } - - function copyRow(shownText, copyValue, variant) { - const row = el("div", "emdb-copy" + (variant ? " " + variant : "")); - row.appendChild(el("code", "emdb-code", esc(shownText))); - const btn = el("button", "emdb-copy-btn", "Copy"); - btn.addEventListener("click", (event) => { - event.stopPropagation(); - copyText(copyValue, btn); - }); - row.appendChild(btn); - return row; - } - - // --- downloads / toasts --------------------------------------------- - function startDownload(name, version) { - state.optimistic.add(labelFor(name, version)); - drawList(); - cmd("download", { name, version: version || "" }); - } - - let lastToastSig = ""; - function drawToasts() { - const downloads = model.get("downloads") || {}; - // Drop cancelling markers for toasts that are already gone. - for (const token of [...state.cancelling]) { - if (!(token in downloads)) state.cancelling.delete(token); - } - // Signature of the toast *set* (tokens + cancelling/error state). When only - // byte-progress changed, update numbers in place so the ✕ button isn't - // rebuilt under the cursor (rebuilding it was eating cancel clicks). - const tokens = Object.keys(downloads); - const sig = tokens - .map((t) => t + (downloads[t].error ? ":e" : state.cancelling.has(t) ? ":c" : "")) - .sort().join("|"); - if (sig === lastToastSig) { - for (const t of tokens) { - if (!downloads[t].error && !state.cancelling.has(t)) updateToastProgress(t, downloads[t]); - } - return; - } - lastToastSig = sig; - toastRoot.innerHTML = ""; - for (const [token, dl] of Object.entries(downloads)) { - toastRoot.appendChild(dl.error ? errorToast(token, dl) : progressToast(token, dl)); - } - } - - function updateToastProgress(token, dl) { - let card = null; - for (const c of toastRoot.children) { if (c.dataset.token === token) { card = c; break; } } - if (!card) return; - const pct = dl.total > 0 ? Math.min(100, (100 * dl.done) / dl.total) : null; - const fill = card.querySelector(".emdb-fill"); - if (fill) { - if (pct == null) { fill.classList.add("indet"); fill.style.width = "32%"; } - else { fill.classList.remove("indet"); fill.style.width = pct + "%"; } - } - const bytes = card.querySelector(".emdb-bytes"); - if (bytes) { - bytes.textContent = pct == null ? `${fmtMB(dl.done)} MB` - : `${fmtMB(dl.done)} / ${fmtMB(dl.total)} MB · ${pct.toFixed(0)}%`; - } - } - - function progressToast(token, dl) { - const cancelling = state.cancelling.has(token); - const pct = dl.total > 0 ? Math.min(100, (100 * dl.done) / dl.total) : null; - const card = el("div", "emdb-toast" + (cancelling ? " cancelling" : "")); - card.dataset.token = token; - const bar = pct == null || cancelling - ? `
` - : `
`; - const bytes = cancelling - ? "Cancelling…" - : (pct == null - ? `${fmtMB(dl.done)} MB` - : `${fmtMB(dl.done)} / ${fmtMB(dl.total)} MB · ${pct.toFixed(0)}%`); - card.innerHTML = - `
${esc(dl.label)}` + - `
` + - `
${bar}
` + - `
${bytes}
`; - // Send the cancel FIRST (so a redraw can't drop it), and keep the button - // clickable so a second click re-sends if the first didn't land. - card.querySelector(".emdb-x").addEventListener("click", () => { - cmd("cancel", { token }); - state.cancelling.add(token); - drawToasts(); - }); - return card; - } - - function errorToast(token, dl) { - const card = el("div", "emdb-toast error"); - card.innerHTML = - `
Failed: ${esc(dl.label)}` + - `
` + - `
${esc(dl.error)}
`; - card.querySelector(".emdb-x").addEventListener("click", () => { - cmd("dismiss", { token }); - }); - return card; - } - - // --- events ---------------------------------------------------------- listEl.addEventListener("mouseleave", () => { state.hovered = null; drawDetails(); }); const onGroups = () => { drawTabs(); drawList(); }; - const onCounts = () => drawHeader(); - const onDownloads = () => { - // A confirmed state change clears the optimistic guesses. - state.optimistic.clear(); - const sig = [...activeNames()].sort().join("|"); - if (sig !== state.activeSig) { // membership changed -> refresh buttons - state.activeSig = sig; - drawList(); - } - drawToasts(); - }; + const onDownloads = () => downloadsChanged(view, toasts); model.on("change:groups", onGroups); - model.on("change:n_downloaded", onCounts); - model.on("change:n_total", onCounts); + model.on("change:n_downloaded", drawCount); + model.on("change:n_total", drawCount); model.on("change:downloads", onDownloads); - // --- first paint ----------------------------------------------------- - drawHeader(); + drawCount(); drawTabs(); - drawList(); - drawToasts(); + onDownloads(); // draws the list, the details panel and the toasts - // Cleanup when the widget view goes away. return () => { - toastRoot.remove(); model.off("change:groups", onGroups); - model.off("change:n_downloaded", onCounts); - model.off("change:n_total", onCounts); + model.off("change:n_downloaded", drawCount); + model.off("change:n_total", drawCount); model.off("change:downloads", onDownloads); }; } diff --git a/emdatabase/static/card.js b/emdatabase/static/card.js index a4970ee..afc55e5 100644 --- a/emdatabase/static/card.js +++ b/emdatabase/static/card.js @@ -1,256 +1,24 @@ -// card.js - frontend for a single dataset's card (DownloadableDataset display). +// card.js - frontend for one dataset's card, what display(ds) shows; +// widget.py puts common.js first. // -// The same content as the browser's details panel, standalone: metadata, -// description, a copy-to-load block, and a Download/Delete button with inline -// progress. Shares browser.css for styling. - -const MB = 1e6; - -function fmtMB(bytes) { - const mb = bytes / MB; - if (mb >= 100) return mb.toFixed(0); - if (mb >= 10) return mb.toFixed(1); - return mb.toFixed(2); -} - -function esc(value) { - return String(value).replace(/[&<>"']/g, (c) => ( - { "&": "&", "<": "<", ">": ">", '"': """, "'": "'" }[c] - )); -} - -function el(tag, className, html) { - const node = document.createElement(tag); - if (className) node.className = className; - if (html != null) node.innerHTML = html; - return node; -} - -function copyText(text, btn) { - const done = () => { - const old = btn.textContent; - btn.textContent = "Copied!"; - btn.classList.add("copied"); - setTimeout(() => { btn.textContent = old; btn.classList.remove("copied"); }, 1100); - }; - if (navigator.clipboard && navigator.clipboard.writeText) { - navigator.clipboard.writeText(text).then(done).catch(() => fallbackCopy(text, done)); - } else { - fallbackCopy(text, done); - } -} - -function fallbackCopy(text, done) { - const ta = document.createElement("textarea"); - ta.value = text; - ta.style.position = "fixed"; - ta.style.opacity = "0"; - document.body.appendChild(ta); - ta.select(); - try { document.execCommand("copy"); done(); } catch (e) { /* ignore */ } - ta.remove(); -} +// The same content as the browser's details panel, standalone, with running +// downloads shown inline under the status line. Shares browser.css for styling. function render({ model, el: root }) { - root.classList.add("emdb", "emdb-card-host"); + root.classList.add("emdb"); const card = el("div", "emdb-card"); root.appendChild(card); + const toasts = el("div", "emdb-toasts"); + const view = newView(model, () => drawEntry(card, model.get("info"), view, toasts)); - let nonce = 0; - let cancelling = false; - let downloadingNow = false; - // Which version of a weights family the card is showing: "" is latest. - let version = ""; - function cmd(action, extra) { - model.set("_command", Object.assign({ action, nonce: nonce++ }, extra || {})); - model.save_changes(); - } - - function copyRow(shownText, copyValue, variant) { - const row = el("div", "emdb-copy" + (variant ? " " + variant : "")); - row.appendChild(el("code", "emdb-code", esc(shownText))); - const btn = el("button", "emdb-copy-btn", "Copy"); - btn.addEventListener("click", (event) => { event.stopPropagation(); copyText(copyValue, btn); }); - row.appendChild(btn); - return row; - } - - // The download state of one version. An item's own fields describe latest, - // so the same reads work for either. - function versionState(it, want) { - if (!want) return it; - return (it.versions || []).find((v) => v.version === want) || {}; - } - - // The version picker: `latest`, then the dated snapshots, ● on the ones - // already on disk. The button, the path and the load snippet follow it. - function versionSelect(it) { - const select = el("select", "emdb-version"); - select.title = "Which version to download, delete or load"; - const choices = [["", "latest"]].concat( - (it.versions || []).map((v) => [v.version, v.version]) - ); - for (const [value, text] of choices) { - const option = document.createElement("option"); - option.value = value; - option.textContent = text + (versionState(it, value).downloaded ? " ●" : ""); - option.selected = value === version; - select.appendChild(option); - } - select.addEventListener("change", () => { version = select.value; draw(); }); - return select; - } - - function progressBox(dl) { - const pct = dl.total > 0 ? Math.min(100, (100 * dl.done) / dl.total) : null; - const box = el("div", "emdb-inline-toast" + (cancelling ? " cancelling" : "")); - const bar = (pct == null || cancelling) - ? `
` - : `
`; - const bytes = cancelling - ? "Cancelling…" - : (pct == null ? `${fmtMB(dl.done)} MB` - : `${fmtMB(dl.done)} / ${fmtMB(dl.total)} MB · ${pct.toFixed(0)}%`); - box.innerHTML = - `
${bytes}` + - `
` + - `
${bar}
`; - // Send cancel first; keep the button clickable so a retry re-sends. - box.querySelector(".emdb-x").addEventListener("click", () => { - cmd("cancel"); cancelling = true; draw(); - }); - return box; - } - - function errorBox(dl) { - const box = el("div", "emdb-inline-toast error"); - box.innerHTML = - `
${esc(dl.error)}` + - `
`; - box.querySelector(".emdb-x").addEventListener("click", () => cmd("dismiss")); - return box; - } - - function draw() { - const it = model.get("info") || {}; - const dl = model.get("download") || {}; - const downloading = dl.done != null && !dl.error; - downloadingNow = downloading; - card.innerHTML = ""; - - if (!(it.versions || []).some((v) => v.version === version)) version = ""; - const where = versionState(it, version); - - const title = el("div", "emdb-d-title", esc(it.name || "")); - if (it.kind === "weights") title.appendChild(el("span", "emdb-kind", "weights")); - if ((it.versions || []).length) title.appendChild(versionSelect(it)); - card.appendChild(title); - const sub = [(it.technique || []).join(", "), it.size, it.shape] - .filter(Boolean).join(" · "); - card.appendChild(el("div", "emdb-d-sub", esc(sub))); - - const status = el("div", "emdb-d-status"); - if (downloading) { - status.appendChild(el("span", "emdb-d-badge", "downloading…")); - } else if (where.downloaded && where.location && where.location !== "personal") { - // `location` is the name of the shared location the copy was found in. - const label = "● " + where.location + (where.user_path ? " + yours" : ""); - const badge = el("span", "emdb-d-badge shared", esc(label)); - badge.title = "from " + where.location + ": " + where.path - + (where.user_path ? "\nyour copy: " + where.user_path : ""); - status.appendChild(badge); - if (where.user_path) { - const del = el("button", "emdb-delete", "Delete yours"); - del.title = "Remove your copy (" + where.user_path + "). The copy in " - + where.location + " is untouched."; - del.addEventListener("click", () => cmd("delete", { version })); - status.appendChild(del); - } - } else if (where.downloaded) { - status.appendChild(el("span", "emdb-d-badge on", "● downloaded")); - const del = el("button", "emdb-delete", "Delete"); - del.title = "Remove the downloaded file from disk"; - del.addEventListener("click", () => cmd("delete", { version })); - status.appendChild(del); - } else { - const btn = el("button", "emdb-dl", "Download"); - btn.addEventListener("click", () => { btn.disabled = true; btn.textContent = "starting…"; cmd("download", { version }); }); - status.appendChild(btn); - } - card.appendChild(status); - - if (downloading) card.appendChild(progressBox(dl)); - else if (dl.error) card.appendChild(errorBox(dl)); - - // Two columns: description + load on the left (wide), metadata on the right. - const cols = el("div", "emdb-card-cols"); - const main = el("div", "emdb-card-col-main"); - const side = el("div", "emdb-card-col-side"); - - if (it.description) main.appendChild(el("p", "emdb-d-desc", esc(it.description))); - main.appendChild(el("div", "emdb-load-label", "Load")); - const snippet = `path = emdatabase.data.${it.name}().download(` - + (version ? `version="${version}"` : "") + ")"; - main.appendChild(copyRow(snippet, snippet)); - if (where.downloaded && where.path) main.appendChild(copyRow(where.path, where.path, "path")); - - const pairs = [ - ["Detector", it.detector], ["Microscope", it.microscope], ["Voltage", it.voltage], - ["Tags", (it.tags || []).join(", ")], ["Authors", (it.authors || []).join(", ")], - ["License", it.license], ["File", it.file], ["DOI", it.doi], - ["Versions", (it.versions || []).map((v) => v.version).join(", ")], - ["Model", it.model_class], - ["Framework", it.model_framework], ["quantem", it.model_quantem], - ]; - const meta = el("div", "emdb-d-meta"); - for (const [key, value] of pairs) { - if (!value) continue; - const kv = el("div", "emdb-kv"); - kv.appendChild(el("span", "emdb-k", key)); - kv.appendChild(el("span", "emdb-v", esc(value))); - meta.appendChild(kv); - } - side.appendChild(meta); - - cols.appendChild(main); - cols.appendChild(side); - card.appendChild(cols); - } - - // Update the progress bar/bytes IN PLACE so the ✕ button is never rebuilt - // mid-download (rebuilding it under the cursor was eating cancel clicks). - function updateProgress(dl) { - const box = card.querySelector(".emdb-inline-toast"); - if (!box || cancelling) { draw(); return; } - const pct = dl.total > 0 ? Math.min(100, (100 * dl.done) / dl.total) : null; - const fill = box.querySelector(".emdb-fill"); - if (fill) { - if (pct == null) { fill.classList.add("indet"); fill.style.width = "32%"; } - else { fill.classList.remove("indet"); fill.style.width = pct + "%"; } - } - const bytesEl = box.querySelector(".emdb-bytes"); - if (bytesEl) { - bytesEl.textContent = pct == null ? `${fmtMB(dl.done)} MB` - : `${fmtMB(dl.done)} / ${fmtMB(dl.total)} MB · ${pct.toFixed(0)}%`; - } - } - - const onInfo = () => { cancelling = false; draw(); }; - const onDownload = () => { - const dl = model.get("download") || {}; - const downloading = dl.done != null && !dl.error; - // A byte-progress tick while already downloading: update numbers only. - if (downloading && downloadingNow && !cancelling) { updateProgress(dl); return; } - if (!downloading) cancelling = false; // cleared/failed -> reset - draw(); - }; - model.on("change:info", onInfo); - model.on("change:download", onDownload); - draw(); + const onDownloads = () => downloadsChanged(view, toasts); + model.on("change:info", view.redraw); + model.on("change:downloads", onDownloads); + onDownloads(); // draws the card and its downloads return () => { - model.off("change:info", onInfo); - model.off("change:download", onDownload); + model.off("change:info", view.redraw); + model.off("change:downloads", onDownloads); }; } diff --git a/emdatabase/static/common.js b/emdatabase/static/common.js new file mode 100644 index 0000000..4371566 --- /dev/null +++ b/emdatabase/static/common.js @@ -0,0 +1,280 @@ +// common.js - what browser.js and card.js share. +// +// anywidget loads each widget's `_esm` as a single module and there is no +// bundler, so widget.py puts this file in front of each widget's own file +// rather than having them import it. + +const MB = 1e6; + +function fmtMB(bytes) { + const mb = bytes / MB; + if (mb >= 100) return mb.toFixed(0); + if (mb >= 10) return mb.toFixed(1); + return mb.toFixed(2); +} + +function esc(value) { + return String(value).replace(/[&<>"']/g, (c) => ( + { "&": "&", "<": "<", ">": ">", '"': """, "'": "'" }[c] + )); +} + +function el(tag, className, html) { + const node = document.createElement(tag); + if (className) node.className = className; + if (html != null) node.innerHTML = html; + return node; +} + +// Copy text to the clipboard (clipboard API, with a textarea fallback for +// non-secure contexts) and briefly flash the button so the click is felt. +function copyText(text, btn) { + const done = () => { + const old = btn.textContent; + btn.textContent = "Copied!"; + btn.classList.add("copied"); + setTimeout(() => { btn.textContent = old; btn.classList.remove("copied"); }, 1100); + }; + if (navigator.clipboard && navigator.clipboard.writeText) { + navigator.clipboard.writeText(text).then(done).catch(() => fallbackCopy(text, done)); + } else { + fallbackCopy(text, done); + } +} + +function fallbackCopy(text, done) { + const ta = document.createElement("textarea"); + ta.value = text; + ta.style.position = "fixed"; + ta.style.opacity = "0"; + document.body.appendChild(ta); + ta.select(); + try { document.execCommand("copy"); done(); } catch (e) { /* ignore */ } + ta.remove(); +} + +// --- a widget view ----------------------------------------------------- + +// What one view of a widget keeps between draws. `cmd` sends a command to +// Python through the synced `_command` trait; the nonce makes a repeated +// action (e.g. two cancels) still register as a change. +function newView(model, redraw) { + let nonce = 0; + return { + model, + redraw, + cmd(action, extra) { + model.set("_command", Object.assign({ action, nonce: nonce++ }, extra)); + model.save_changes(); + }, + optimistic: new Set(), // labels just clicked, before Python confirms + versions: {}, // name -> chosen version of a weights family ("" = latest) + activeSig: null, // the running downloads at the last redraw + }; +} + +// A download is labelled "Name" or "Name@260902", as widget.py names it. +function labelFor(name, version) { + return version ? name + "@" + version : name; +} + +function activeLabels(view) { + const labels = new Set(view.optimistic); + for (const dl of Object.values(view.model.get("downloads"))) { + if (!dl.error) labels.add(dl.label); + } + return labels; +} + +function startDownload(view, name, version) { + view.optimistic.add(labelFor(name, version)); + view.redraw(); + view.cmd("download", { name, version }); +} + +// `downloads` changed, so Python has confirmed or refused what was clicked. +// Redraw only if the set of running downloads changed; a byte count changing +// needs just the toasts. +function downloadsChanged(view, stack) { + view.optimistic.clear(); + const sig = [...activeLabels(view)].sort().join("|"); + if (sig !== view.activeSig) { + view.activeSig = sig; + view.redraw(); + } + drawToasts(stack, view); +} + +// --- download progress ----------------------------------------------- + +// One toast per entry of `downloads`, drawn into `stack` inside the widget: +// progress and a cancel button while it runs, the error and a dismiss button +// once it has failed. The toasts are rebuilt only when that set changes; a +// byte count is updated in place, so the ✕ button is not rebuilt under the +// cursor (which was eating cancel clicks). +function drawToasts(stack, view) { + const downloads = view.model.get("downloads"); + const cancelling = (stack.cancelling ||= new Set()); + for (const token of cancelling) if (!(token in downloads)) cancelling.delete(token); + const sig = Object.keys(downloads) + .map((t) => t + (downloads[t].error ? ":e" : cancelling.has(t) ? ":c" : "")) + .sort().join("|"); + if (stack.dataset.sig !== sig) { + stack.dataset.sig = sig; + stack.innerHTML = ""; + for (const [token, dl] of Object.entries(downloads)) { + const toast = el("div", "emdb-toast" + + (dl.error ? " error" : cancelling.has(token) ? " cancelling" : "")); + toast.dataset.token = token; + toast.innerHTML = + `
` + + esc((dl.error ? "Failed: " : "") + dl.label) + `` + + `
` + + (dl.error + ? `
${esc(dl.error)}
` + : `
`); + toast.querySelector(".emdb-x").addEventListener("click", () => { + if (dl.error) { view.cmd("dismiss", { token }); return; } + // Send the cancel first, so a redraw can't drop it, and keep the button + // clickable so a second click re-sends if the first didn't land. + view.cmd("cancel", { token }); + cancelling.add(token); + drawToasts(stack, view); + }); + stack.appendChild(toast); + } + } + for (const toast of stack.children) { + const dl = downloads[toast.dataset.token]; + if (dl.error) continue; + const pct = cancelling.has(toast.dataset.token) || !(dl.total > 0) + ? null : Math.min(100, (100 * dl.done) / dl.total); + const fill = toast.querySelector(".emdb-fill"); + fill.classList.toggle("indet", pct == null); + fill.style.width = pct == null ? "32%" : pct + "%"; + toast.querySelector(".emdb-bytes").textContent = + cancelling.has(toast.dataset.token) ? "Cancelling…" + : pct == null ? `${fmtMB(dl.done)} MB` + : `${fmtMB(dl.done)} / ${fmtMB(dl.total)} MB · ${pct.toFixed(0)}%`; + } +} + +// --- one catalogue row ------------------------------------------------- + +// The download state of one version: an item's own fields describe latest, so +// the same reads work for the item or for one of its `versions`. +function versionState(item, version) { + if (!version) return item; + return item.versions.find((v) => v.version === version) || {}; +} + +// `location` names the location a copy was found in; "personal" is the user's +// own directory. +function inShared(where) { + return where.downloaded && where.location !== "personal"; +} + +// A shared copy and your own can both exist; the tooltip names each. +function sharedTitle(where) { + return "from " + where.location + ": " + where.path + + (where.user_path ? "\nyour copy: " + where.user_path : ""); +} + +function copyRow(text, variant) { + const row = el("div", "emdb-copy" + (variant ? " " + variant : "")); + row.appendChild(el("code", "emdb-code", esc(text))); + const btn = el("button", "emdb-copy-btn", "Copy"); + btn.addEventListener("click", () => copyText(text, btn)); + row.appendChild(btn); + return row; +} + +// Draw `item` into `parent`: what the browser's details panel and a dataset's +// card both show. A weights family gets a version picker - `latest`, then the +// dated snapshots, ● on those on disk - and the status, the path and the load +// snippet all follow it. `toasts`, if given, goes under the status line. +function drawEntry(parent, item, view, toasts) { + const chosen = view.versions[item.name]; + const version = item.versions.some((v) => v.version === chosen) ? chosen : ""; + const where = versionState(item, version); + parent.innerHTML = ""; + + const title = el("div", "emdb-d-title", esc(item.name)); + if (item.kind === "weights") title.appendChild(el("span", "emdb-kind", "weights")); + if (item.versions.length) { + const select = el("select", "emdb-version"); + select.title = "Which version to download, delete or load"; + for (const value of ["", ...item.versions.map((v) => v.version)]) { + const option = document.createElement("option"); + option.value = value; + option.textContent = (value || "latest") + (versionState(item, value).downloaded ? " ●" : ""); + option.selected = value === version; + select.appendChild(option); + } + select.addEventListener("change", () => { + view.versions[item.name] = select.value; + view.redraw(); + }); + title.appendChild(select); + } + parent.appendChild(title); + const sub = [item.technique.join(", "), item.size].filter(Boolean).join(" · "); + parent.appendChild(el("div", "emdb-d-sub", esc(sub))); + + const status = el("div", "emdb-d-status"); + const deleteButton = (text, tooltip) => { + const del = el("button", "emdb-delete", text); + del.title = tooltip; + del.addEventListener("click", () => view.cmd("delete", { name: item.name, version })); + status.appendChild(del); + }; + if (activeLabels(view).has(labelFor(item.name, version))) { + status.appendChild(el("span", "emdb-d-badge", "downloading…")); + } else if (inShared(where)) { + const label = "● " + where.location + (where.user_path ? " + yours" : ""); + const badge = el("span", "emdb-d-badge shared", esc(label)); + badge.title = sharedTitle(where); + status.appendChild(badge); + if (where.user_path) { + deleteButton("Delete yours", "Remove your copy (" + where.user_path + "). The copy in " + + where.location + " is untouched."); + } + } else if (where.downloaded) { + status.appendChild(el("span", "emdb-d-badge on", "● downloaded")); + deleteButton("Delete", "Remove the downloaded file from disk"); + } else { + const btn = el("button", "emdb-dl", "Download"); + btn.addEventListener("click", () => startDownload(view, item.name, version)); + status.appendChild(btn); + } + parent.appendChild(status); + if (toasts) parent.appendChild(toasts); + + const main = el("div", "emdb-d-main"); + if (item.description) main.appendChild(el("p", "emdb-d-desc", esc(item.description))); + main.appendChild(el("div", "emdb-load-label", "Load")); + main.appendChild(copyRow(`path = emdatabase.data.${item.name}().download(` + + (version ? `version="${version}"` : "") + ")")); + if (where.path) main.appendChild(copyRow(where.path, "path")); + + const meta = el("div", "emdb-d-meta"); + const pairs = [ + ["Detector", item.detector], ["Microscope", item.microscope], ["Voltage", item.voltage], + ["Tags", item.tags.join(", ")], ["Authors", item.authors.join(", ")], + ["License", item.license], ["File", item.file], ["DOI", item.doi], + ["Versions", item.versions.map((v) => v.version).join(", ")], + ["Model", item.model_class], ["Framework", item.model_framework], + ["quantem", item.model_quantem], + ]; + for (const [key, value] of pairs) { + if (!value) continue; + const kv = el("div", "emdb-kv"); + kv.appendChild(el("span", "emdb-k", key)); + kv.appendChild(el("span", "emdb-v", esc(value))); + meta.appendChild(kv); + } + + const cols = el("div", "emdb-d-cols"); + cols.append(main, meta); + parent.appendChild(cols); +} diff --git a/emdatabase/static/toasts.js b/emdatabase/static/toasts.js deleted file mode 100644 index 5b6effa..0000000 --- a/emdatabase/static/toasts.js +++ /dev/null @@ -1,144 +0,0 @@ -// toasts.js - a global, fixed bottom-right download-toast stack. -// -// Backs the toasts that a bare `ds.download()` pops up in Jupyter. The widget -// itself is an invisible anchor; the toasts live on (position: fixed) so -// they float over the notebook regardless of which cell started the download. -// -// The widget is re-displayed on every download (so it re-anchors after a cell -// is cleared/re-run), which means several views of the same model can be live -// at once. They therefore SHARE one body-level toast root (by id), one -// "cancelling" set, and one render signature, so they never duplicate or -// clobber each other; the root is ref-counted and removed with the last view. - -const MB = 1e6; - -function fmtMB(bytes) { - const mb = bytes / MB; - if (mb >= 100) return mb.toFixed(0); - if (mb >= 10) return mb.toFixed(1); - return mb.toFixed(2); -} - -function esc(value) { - return String(value).replace(/[&<>"']/g, function (c) { - return { "&": "&", "<": "<", ">": ">", '"': """, "'": "'" }[c]; - }); -} - -function el(tag, cls, html) { - const n = document.createElement(tag); - if (cls) n.className = cls; - if (html != null) n.innerHTML = html; - return n; -} - -function render({ model, el: root }) { - root.style.display = "none"; // the widget is just an anchor - - // One shared toast root for every live view of the (singleton) model. - let toastRoot = document.getElementById("emdb-global-toast-root"); - if (!toastRoot) { - toastRoot = el("div", "emdb-toast-root"); - toastRoot.id = "emdb-global-toast-root"; - document.body.appendChild(toastRoot); - } - window.__emdbToastViews = (window.__emdbToastViews || 0) + 1; - const cancelling = (window.__emdbCancelling = window.__emdbCancelling || new Set()); - - let nonce = 0; - function cmd(action, extra) { - model.set("_command", Object.assign({ action, nonce: nonce++ }, extra || {})); - model.save_changes(); - } - - function draw() { - const downloads = model.get("downloads") || {}; - for (const t of [...cancelling]) if (!(t in downloads)) cancelling.delete(t); - const tokens = Object.keys(downloads); - // Shared signature on the root, so only one view rebuilds per set-change. - const sig = tokens - .map(function (t) { return t + (downloads[t].error ? ":e" : cancelling.has(t) ? ":c" : ""); }) - .sort().join("|"); - if (toastRoot.dataset.sig === sig) { // only byte-progress changed - for (const t of tokens) { - if (!downloads[t].error && !cancelling.has(t)) update(t, downloads[t]); - } - return; - } - toastRoot.dataset.sig = sig; - toastRoot.innerHTML = ""; - for (const [token, dl] of Object.entries(downloads)) { - toastRoot.appendChild(dl.error ? errorToast(token, dl) : progressToast(token, dl)); - } - } - - function update(token, dl) { - let card = null; - for (const c of toastRoot.children) { if (c.dataset.token === token) { card = c; break; } } - if (!card) return; - const pct = dl.total > 0 ? Math.min(100, (100 * dl.done) / dl.total) : null; - const fill = card.querySelector(".emdb-fill"); - if (fill) { - if (pct == null) { fill.classList.add("indet"); fill.style.width = "32%"; } - else { fill.classList.remove("indet"); fill.style.width = pct + "%"; } - } - const bytes = card.querySelector(".emdb-bytes"); - if (bytes) { - bytes.textContent = pct == null ? fmtMB(dl.done) + " MB" - : fmtMB(dl.done) + " / " + fmtMB(dl.total) + " MB · " + pct.toFixed(0) + "%"; - } - } - - function progressToast(token, dl) { - const isc = cancelling.has(token); - const pct = dl.total > 0 ? Math.min(100, (100 * dl.done) / dl.total) : null; - const card = el("div", "emdb-toast" + (isc ? " cancelling" : "")); - card.dataset.token = token; - const bar = (pct == null || isc) - ? '
' - : '
'; - const bytes = isc ? "Cancelling…" - : (pct == null ? fmtMB(dl.done) + " MB" - : fmtMB(dl.done) + " / " + fmtMB(dl.total) + " MB · " + pct.toFixed(0) + "%"); - card.innerHTML = - '
' + esc(dl.label) + "" + - '
' + - '
' + bar + "
" + - '
' + bytes + "
"; - card.querySelector(".emdb-x").addEventListener("click", function () { - cmd("cancel", { token: token }); - cancelling.add(token); - toastRoot.dataset.sig = ""; // force a rebuild into the cancelling state - draw(); - }); - return card; - } - - function errorToast(token, dl) { - const card = el("div", "emdb-toast error"); - card.dataset.token = token; - card.innerHTML = - '
Failed: ' + esc(dl.label) + "" + - '
' + - '
' + esc(dl.error) + "
"; - card.querySelector(".emdb-x").addEventListener("click", function () { cmd("dismiss", { token: token }); }); - return card; - } - - const onDownloads = function () { draw(); }; - model.on("change:downloads", onDownloads); - // A fresh view must render from scratch (its shared root may hold stale sig). - toastRoot.dataset.sig = ""; - draw(); - - return function () { - model.off("change:downloads", onDownloads); - window.__emdbToastViews -= 1; - if (window.__emdbToastViews <= 0) { - const tr = document.getElementById("emdb-global-toast-root"); - if (tr) tr.remove(); - } - }; -} - -export default { render }; diff --git a/emdatabase/tests/test_load_data.py b/emdatabase/tests/test_load_data.py index ddb7da2..14e9e47 100644 --- a/emdatabase/tests/test_load_data.py +++ b/emdatabase/tests/test_load_data.py @@ -332,12 +332,13 @@ def block(): release.set() -def test_a_caller_s_progress_bar_is_not_replaced_by_the_toast(tmp_path, monkeypatch): - """The Jupyter toast stands in for the default bar, not for one passed in.""" - import emdatabase.widget as widget_mod - +def test_a_background_download_builds_its_bar_on_the_calling_thread(tmp_path, monkeypatch): + """A notebook shows a bar in the cell that was running when it was built, which + a pool thread does not know. A file already on disk gets no bar, and a + caller's own Progress is passed through untouched.""" dataset = getattr(data, TINY_DATASET)() seen = [] + built_on = [] def record(destination=None, progressbar=True, chunk_size=4096, version=None, refresh=False): seen.append(progressbar) @@ -345,11 +346,19 @@ def record(destination=None, progressbar=True, chunk_size=4096, version=None, re target.write_bytes(b"payload") return str(target) - monitor = object() + class Bar: + def __init__(self, **kwargs): + built_on.append(threading.current_thread()) + monkeypatch.setattr(dataset, "_retrieve", record) - monkeypatch.setattr(widget_mod, "_attach_toast", lambda label: (monitor, None)) + monkeypatch.setattr("tqdm.auto.tqdm", Bar) + + dataset.download(destination=tmp_path).wait(2) + assert isinstance(seen[0], _TqdmProgress) + assert built_on == [threading.current_thread()] mine = object() + dataset.download(destination=tmp_path).wait(2) # on disk now dataset.download(destination=tmp_path, progressbar=mine).wait(2) - dataset.download(destination=tmp_path, progressbar=True, refresh=True).wait(2) - assert seen == [mine, monitor] + assert seen[1:] == [True, mine] + assert len(built_on) == 1 diff --git a/emdatabase/tests/test_widget.py b/emdatabase/tests/test_widget.py index b042be5..f22f062 100644 --- a/emdatabase/tests/test_widget.py +++ b/emdatabase/tests/test_widget.py @@ -210,10 +210,13 @@ def test_dataset_card_is_populated_and_routes(monkeypatch): assert widget.info["name"] == TINY_DATASET assert widget.info["technique"] == ["STEM"] calls = [] - monkeypatch.setattr(widget, "_start_download", lambda version=None: calls.append(version)) - widget._command = {"action": "download", "nonce": 1} - widget._command = {"action": "download", "version": "260902", "nonce": 2} + monkeypatch.setattr( + widget, "_start_download", lambda name, version=None: calls.append(version) + ) + widget._command = {"action": "download", "name": TINY_DATASET, "nonce": 1} + widget._command = {"action": "download", "name": TINY_DATASET, "version": "260902", "nonce": 2} assert calls == [None, "260902"] + assert widget._resolve(TINY_DATASET) is ds def test_dataset_display_is_a_widget_card(): @@ -240,41 +243,29 @@ def _boom(_dataset): assert "text/plain" in bundle -def test_attach_toast_is_noop_outside_jupyter(): - """Outside a Jupyter kernel there is no toast, so a bare download is - unaffected.""" - import emdatabase.widget as widget_mod - - monitor, finish = widget_mod._attach_toast("Foo") - assert monitor is None and finish is None - - -def test_download_toasts_plumbing(): - """The global toasts widget tracks a download and clears/errors/cancels it.""" - pytest.importorskip("anywidget") +def test_download_progress_plumbing(): + """A download's progress is tracked, then cleared, errored or cancelled.""" from concurrent.futures import Future - import emdatabase.widget as widget_mod - - toasts = widget_mod._make_toasts_class()() + widget = _browser() - monitor, token = toasts.begin("Foo") - assert toasts.downloads[token] == {"label": "Foo", "done": 0, "total": 0} + monitor, token = widget.begin("Foo") + assert widget.downloads[token] == {"label": "Foo", "done": 0, "total": 0} ok = Future() ok.set_result("path") - toasts.finish(token, ok) # success -> toast cleared - assert token not in toasts.downloads + widget.finish(token, ok) # success -> progress cleared + assert token not in widget.downloads - monitor2, token2 = toasts.begin("Bar") # cancel sets the event - toasts._command = {"action": "cancel", "token": token2, "nonce": 1} + monitor2, token2 = widget.begin("Bar") # cancel sets the event + widget._command = {"action": "cancel", "token": token2, "nonce": 1} assert monitor2._cancel.is_set() bad = Future() bad.set_exception(RuntimeError("boom")) - _, token3 = toasts.begin("Baz") - toasts.finish(token3, bad) # failure -> error toast - assert toasts.downloads[token3]["error"] == "boom" + _, token3 = widget.begin("Baz") + widget.finish(token3, bad) # failure -> error shown + assert widget.downloads[token3]["error"] == "boom" @pytest.mark.slow @@ -308,7 +299,7 @@ def boom(version=None): monkeypatch.setattr(ds, "delete", boom) widget = widget_mod.card(ds) with pytest.warns(UserWarning, match="could not delete"): - widget._on_command({"new": {"action": "delete", "nonce": 1}}) + widget._on_command({"new": {"action": "delete", "name": TINY_DATASET, "nonce": 1}}) def test_module_display_reports_a_real_error_rather_than_install_advice(monkeypatch): diff --git a/emdatabase/widget.py b/emdatabase/widget.py index e4d5747..8d18048 100644 --- a/emdatabase/widget.py +++ b/emdatabase/widget.py @@ -3,8 +3,8 @@ Renders every dataset grouped by technique, marks which are already downloaded (○ / ●), shows metadata on hover, and downloads on click. Each download runs on the shared background thread pool (so the kernel stays responsive) and reports -progress through a toast card with a cancel button - the same idea as a desktop -app's download manager, driven here through pooch's progress-bar hook. +progress, with a cancel button, inside the widget through pooch's progress-bar +hook. anywidget is an optional dependency; :func:`browse` raises a helpful error if it is not installed. Importing this module never imports anywidget at module load, @@ -58,7 +58,7 @@ def _prepare_frontend(): """Everything that should happen before a widget renders. pooch's "Downloading data from ..." INFO logs render as red output in - Jupyter, and the widget shows the same thing as a toast, so they are + Jupyter, and the widget or the progress bar shows the same thing, so they are silenced - once, so a level set afterwards is left alone. Warnings and errors still come through. """ @@ -70,7 +70,7 @@ def _prepare_frontend(): def _label(name, version=None): - """What a download is called in a toast: ``Name`` or ``Name@260902``. + """What a download is called in the widget: ``Name`` or ``Name@260902``. The frontend splits on the ``@`` to find the entry a running download belongs to, so a dated download still marks its row. @@ -78,6 +78,15 @@ def _label(name, version=None): return f"{name}@{version}" if version else name +def _frontend(name): + """A widget's ``_esm``: ``common.js``, which both widgets share, then its own file. + + anywidget loads ``_esm`` as a single module and there is no bundler, so the + shared code is put in front rather than imported. + """ + return (_STATIC / "common.js").read_text("utf-8") + (_STATIC / name).read_text("utf-8") + + class DownloadCancelled(Exception): """Raised inside pooch's stream when the user cancels a download. @@ -153,12 +162,12 @@ def _delete_file(dataset, name, version): @functools.cache def _make_downloads_class(): - """Build the base of the browser and the toasts widget, importing anywidget lazily. + """Build the base of the browser and the card, importing anywidget lazily. Both draw one toast per running download from ``downloads``, which maps a per-download token to ``{label, done, total}`` while it runs and to - ``{label, error}`` once it has failed, and both take the cancel and dismiss - commands those toasts send. + ``{label, error}`` once it has failed, and send the download, delete, + refresh, cancel and dismiss commands. """ import anywidget import traitlets @@ -183,13 +192,48 @@ def __init__(self, **kwargs): # internal comm callback, and overriding it breaks all comm handling # (including trait sync). Commands arrive via the `_command` trait. def _on_command(self, change): - command = change.get("new") or {} + command = change["new"] + action = command.get("action") + name = str(command.get("name", "")) + version = command.get("version") or None token = str(command.get("token", "")) - if command.get("action") == "cancel": + if action == "download": + self._start_download(name, version) + elif action == "delete": + self._delete(name, version) + elif action == "refresh": + self.refresh() + elif action == "cancel": self._cancel(token) - elif command.get("action") == "dismiss": + elif action == "dismiss": self._clear_progress(token) + def refresh(self): + """Re-check which files are on disk.""" + raise NotImplementedError + + def _resolve(self, name): + return _catalogue.resolve(name) + + def _delete(self, name, version=None): + """Delete a dataset's downloaded file and refresh its status.""" + ds = self._resolve(name) + if ds is not None: + _delete_file(ds, name, version) + self.refresh() + + def _start_download(self, name, version=None): + """Kick off a background download for ``name`` and show a toast.""" + ds = self._resolve(name) + if ds is None: + return None + monitor, token = self.begin(_label(name, version)) + future = _get_executor().submit( + ds.download, progressbar=monitor, background=False, version=version + ) + future.add_done_callback(lambda f, tk=token: self.finish(tk, f)) + return future + def begin(self, label): """Register a new download; return its (monitor, token).""" token = f"{label}-{next(self._counter)}" @@ -211,6 +255,7 @@ def finish(self, token, future): self._set_error(token, label, str(error)) else: self._clear_progress(token) + self.refresh() def _cancel(self, token): with self._lock: @@ -250,7 +295,7 @@ def _make_browser_class(): class DatasetBrowser(_make_downloads_class()): """Interactive, hoverable list of the emdatabase datasets.""" - _esm = _STATIC / "browser.js" + _esm = _frontend("browser.js") groups = traitlets.List().tag(sync=True) data_dir = traitlets.Unicode().tag(sync=True) @@ -269,122 +314,31 @@ def refresh(self): self.n_downloaded = cat["n_downloaded"] self.n_total = cat["n_total"] - def _on_command(self, change): - command = change.get("new") or {} - action = command.get("action") - name = str(command.get("name", "")) - version = command.get("version") or None - if action == "download": - self._start_download(name, version) - elif action == "delete": - self._delete(name, version) - elif action == "refresh": - self.refresh() - else: - super()._on_command(change) - - def _delete(self, name, version=None): - """Delete a dataset's downloaded file and refresh its status.""" - ds = _catalogue.resolve(name) - if ds is not None: - _delete_file(ds, name, version) - self.refresh() - - def _start_download(self, name, version=None): - """Kick off a background download for ``name`` and show a toast.""" - ds = _catalogue.resolve(name) - if ds is None: - return None - monitor, token = self.begin(_label(name, version)) - future = _get_executor().submit( - ds.download, progressbar=monitor, background=False, version=version - ) - future.add_done_callback(lambda f, tk=token: self.finish(tk, f)) - return future - - def finish(self, token, future): - super().finish(token, future) - self.refresh() - return DatasetBrowser @functools.cache def _make_card_class(): """Build the ``DatasetCard`` class (one dataset), importing anywidget lazily.""" - import anywidget import traitlets - class DatasetCard(anywidget.AnyWidget): + class DatasetCard(_make_downloads_class()): """An interactive card for a single dataset - what ``display(ds)`` shows.""" - _esm = _STATIC / "card.js" - _css = _STATIC / "browser.css" + _esm = _frontend("card.js") info = traitlets.Dict().tag(sync=True) # the catalogue entry() dict - download = traitlets.Dict().tag(sync=True) # {label, done, total} | {} | {error} - _command = traitlets.Dict().tag(sync=True) def __init__(self, dataset, **kwargs): super().__init__(**kwargs) self._dataset = dataset - self._name = type(dataset).__name__ - self._lock = threading.RLock() - self._cancel = None - self._counter = itertools.count() self.refresh() - self.observe(self._on_command, names="_command") def refresh(self): - self.info = _catalogue.entry(self._name, self._dataset) - - def _on_command(self, change): - command = change.get("new") or {} - action = command.get("action") - version = command.get("version") or None - if action == "download": - self._start_download(version) - elif action == "cancel": - with self._lock: - event = self._cancel - if event is not None: - event.set() - elif action == "dismiss": - self.download = {} - elif action == "delete": - _delete_file(self._dataset, self._name, version) - self.refresh() - elif action == "refresh": - self.refresh() + self.info = _catalogue.entry(type(self._dataset).__name__, self._dataset) - def _start_download(self, version=None): - with self._lock: - if self._cancel is not None: - return # already downloading - self._cancel = threading.Event() - label = _label(self._name, version) - token = f"{label}-{next(self._counter)}" - self.download = {"label": label, "done": 0, "total": 0} - monitor = _WidgetProgress(self, token, label, self._cancel) - future = _get_executor().submit( - self._dataset.download, progressbar=monitor, background=False, version=version - ) - future.add_done_callback(self._finish_download) - return future - - def _finish_download(self, future): - with self._lock: - self._cancel = None - error = future.exception() - if error is not None and not isinstance(error, DownloadCancelled): - self.download = {"label": self._name, "error": str(error)} - else: - self.download = {} - self.refresh() - - # Called from the worker thread by _WidgetProgress. - def _set_progress(self, token, label, done, total): - self.download = {"label": label, "done": int(done), "total": int(total)} + def _resolve(self, name): + return self._dataset return DatasetCard @@ -424,26 +378,6 @@ def browse(**kwargs): return cls(**kwargs) -# --------------------------------------------------------------------------- -# Global toasts: a bare ``ds.download()`` in Jupyter pops a cancelable toast -# --------------------------------------------------------------------------- - - -@functools.cache -def _make_toasts_class(): - """Build the singleton ``DownloadToasts`` widget, importing anywidget lazily.""" - - class DownloadToasts(_make_downloads_class()): - """An invisible anchor that floats download toasts at the viewport corner.""" - - _esm = _STATIC / "toasts.js" - - return DownloadToasts - - -_toasts = None - - def _in_notebook(): """True in a notebook frontend that can render widgets (Jupyter, Colab, VS Code, ...), False in plain Python or a terminal IPython.""" @@ -458,40 +392,3 @@ def _in_notebook(): return ip.__class__.__name__ != "TerminalInteractiveShell" except Exception: return False - - -def _get_toasts(): - """Return the singleton toasts widget, or None if a toast can't be shown - (not in a notebook, or anywidget missing). - - The widget is re-displayed on every call so it re-anchors in the current - cell: a widget view is tied to a cell's output, so clearing or re-running - that cell kills the view. Re-displaying gives a fresh, live view each time; - the views share one body-level toast root (see toasts.js), so re-anchoring - never duplicates the toasts. - """ - global _toasts - if not _in_notebook(): - return None - _prepare_frontend() - try: - if _toasts is None: - _toasts = _make_toasts_class()() - from IPython.display import display - - display(_toasts) - except Exception: - return None - return _toasts - - -def _attach_toast(label): - """If a toast can be shown, return (monitor, finish_callback) for a new - download; otherwise (None, None). The monitor is a pooch progress object - that also honors cancellation; finish_callback(future) clears the toast. - """ - toasts = _get_toasts() - if toasts is None: - return None, None - monitor, token = toasts.begin(label) - return monitor, (lambda future, tk=token, tw=toasts: tw.finish(tk, future)) From 4996bef6d1522538e2a894c627f632a3081d71b0 Mon Sep 17 00:00:00 2001 From: arthurmccray Date: Tue, 15 Sep 2026 12:10:05 -0700 Subject: [PATCH 14/18] cleaning up docs --- docs/source/_build_docs.py | 918 +++++---------------------- docs/source/add_dataset.rst | 4 +- docs/source/all_data.rst | 4 +- docs/source/conf.py | 68 +- docs/source/datasets.rst | 15 - docs/source/index.rst | 6 +- docs/source/intro.rst | 54 -- docs/source/weights.rst | 5 +- emdatabase/static/browser.js | 33 +- emdatabase/static/common.js | 129 +++- emdatabase/tests/test_issue_route.py | 42 +- 11 files changed, 285 insertions(+), 993 deletions(-) delete mode 100644 docs/source/datasets.rst delete mode 100644 docs/source/intro.rst diff --git a/docs/source/_build_docs.py b/docs/source/_build_docs.py index e06ace3..dcdb5c4 100644 --- a/docs/source/_build_docs.py +++ b/docs/source/_build_docs.py @@ -1,634 +1,104 @@ +"""The docs site's app pages, written over Sphinx's output by ``conf.py``. + +Every page is self-contained: the widget's CSS and shared JS are inlined, the +catalogue is baked in as JSON at build time, and nothing is loaded from +outside, so search and the list work with no backend. +""" + import json -from collections import defaultdict +from html import escape from importlib import resources from pathlib import Path -import yaml +from emdatabase.metadata import acquisition_techniques, versioned_filename -from emdatabase.metadata import NON_DATASET_FILES, acquisition_techniques - - -def parse_datasets(yaml_dir): - """Parse all YAML files and organize by technique. - - An entry may declare several techniques, in which case it is listed under - each of them; ``techniques`` on the record is all of them, so the table can - still draw it as one row. - """ - datasets_by_technique = defaultdict(list) - - for yaml_file in sorted(Path(yaml_dir).glob("*.yaml")): - if yaml_file.name in NON_DATASET_FILES: - continue - with open(yaml_file, "r") as f: - data = yaml.safe_load(f) - - for name, info in data.items(): - if info.get("kind") == "weights": - continue # the Model Weights page, not this one - techniques = info.get("technique") or ["Unknown"] - if isinstance(techniques, str): - techniques = [techniques] - record = { - "name": name, - "techniques": list(techniques), - "description": info.get("description", ""), - "tags": info.get("tags", []), - "source": info.get("source", ""), - "file": info.get("file", ""), - "license": info.get("license", ""), - "detector": info.get("detector", "Unknown"), - "detector_manufacturer": info.get("detector_manufacturer", "Unknown"), - } - for technique in techniques: - datasets_by_technique[technique].append(record) - - return dict(datasets_by_technique) - - -def generate_html_table(datasets_by_technique): - """Generate HTML with filterable table and technique tabs.""" - from emdatabase import catalogue - - all_tags = set() - all_detectors = {} # Changed to dict: {manufacturer: [detectors]} - technique_tags = {} - technique_detectors = {} - - for technique, datasets in datasets_by_technique.items(): - tags = set() - detectors = {} - for dataset in datasets: - tags.update(dataset["tags"]) - all_tags.update(dataset["tags"]) - manufacturer = dataset.get("detector_manufacturer", "Unknown") - detector = dataset.get("detector", "Unknown") - - if manufacturer not in detectors: - detectors[manufacturer] = set() - detectors[manufacturer].add(detector) - - if manufacturer not in all_detectors: - all_detectors[manufacturer] = set() - all_detectors[manufacturer].add(detector) - - technique_tags[technique] = sorted(tags) - technique_detectors[technique] = {m: sorted(d) for m, d in detectors.items()} - - all_detectors = {m: sorted(d) for m, d in all_detectors.items()} - - technique_tabs_json = json.dumps(catalogue.ordered_groups(datasets_by_technique)) - technique_tags_json = __import__("json").dumps(technique_tags) - technique_detectors_json = __import__("json").dumps(technique_detectors) - all_tags_sorted = sorted(all_tags) - all_detectors_json = __import__("json").dumps(all_detectors) - - html = """ - - - - - - - -

EM Datasets

- -
- -
- - - - - - - - - - - - - - - """ - - # A dataset declaring several techniques is under each of their keys, so it - # gets one row carrying all of them and the tabs filter on membership. - written = set() - for technique in sorted(datasets_by_technique.keys()): - for dataset in datasets_by_technique[technique]: - if dataset["name"] in written: - continue - written.add(dataset["name"]) - tags_str = ", ".join(dataset["tags"]) - techniques_str = ", ".join(dataset["techniques"]) - manufacturer = dataset.get("detector_manufacturer", "Unknown") - detector = dataset.get("detector", "Unknown") - detector_full = f"{manufacturer} - {detector}" - html += f""" - - - - - - - - - """ - - html += f""" -
TechniqueDatasetDescription - Tags -
- ▼ -
-
- 		- Detector -
- ▼ -
-
- 		FileLicense
{techniques_str}{dataset["name"]}{dataset["description"]}{tags_str}{detector_full}{dataset["file"]}{dataset["license"]}
- - - - """ - return html - - -# --------------------------------------------------------------------------- -# Widget-styled browser for the docs landing page -# --------------------------------------------------------------------------- -# -# Reuses the Jupyter widget's CSS (emdatabase/static/browser.css) and the -# emdatabase.catalogue data model so the docs page looks and browses exactly -# like emdatabase.browse(). A static site has no kernel, so instead of live -# downloads the details panel offers the copy-to-load snippet and a direct link -# to the source file. +_STATIC = resources.files("emdatabase") / "static" +# The browser on the landing, All Data and Model Weights pages: the widget's +# layout over DATA, with its tab list TABS and what it lists, LABEL, all baked in +# next to it. A static site has no kernel, so instead of Download and Delete the +# details panel offers the load snippet and a direct link to the file. +# common.js is put in front of it. _DOCS_BROWSER_JS = r""" -(function () { - var root = document.getElementById("root"); +(() => { + const root = document.getElementById("root"); root.classList.add("emdb"); - // What this page is browsing; baked in next to DATA so one script serves the - // dataset pages and the weights page. - var WHAT = (typeof LABEL !== "undefined" && LABEL) ? LABEL : "Datasets"; - // `version` maps a weights family to the version being shown ("" = latest). - var state = { tab: "All", search: "", selected: null, hovered: null, version: {} }; - - function esc(v) { - return String(v).replace(/[&<>"']/g, function (c) { - return { "&": "&", "<": "<", ">": ">", '"': """, "'": "'" }[c]; - }); + const what = LABEL.toLowerCase(); + const state = { tab: "All", search: "", selected: null, hovered: null }; + const view = { versions: {}, redraw: () => drawDetails() }; + const allItems = DATA.groups.flatMap((g) => g.items); + + const header = el("div", "emdb-header"); + const top = el("div", "emdb-header-top", `
${esc(LABEL)}
`); + top.appendChild(el("div", "emdb-count", `${DATA.n_total} ${what}`)); + const search = el("input", "emdb-search"); + search.type = "text"; + search.placeholder = `Search ${what}…`; + search.addEventListener("input", () => { state.search = search.value; drawList(); }); + header.append(top, search); + const tabsEl = el("div", "emdb-tabs"); + const listEl = el("div", "emdb-list"); + const detailsEl = el("div", "emdb-details"); + const body = el("div", "emdb-body"); + body.append(listEl, detailsEl); + root.append(header, tabsEl, body); + + function drawRow(item) { + const row = el("div", "emdb-row" + (state.selected === item.name ? " selected" : "")); + row.append(el("span", "emdb-glyph off", "•"), el("span", "emdb-name", esc(item.name)), + el("span", "emdb-meta", esc(item.size))); + row.addEventListener("mouseenter", () => { state.hovered = item.name; drawDetails(); }); + row.addEventListener("click", () => { state.selected = item.name; drawList(); }); + return row; } - function el(tag, cls, html) { - var n = document.createElement(tag); - if (cls) n.className = cls; - if (html != null) n.innerHTML = html; - return n; + + function drawList() { + fillList(listEl, DATA.groups, state, drawRow, `No ${what} match.`); + state.selected ??= allItems[0]?.name; + drawDetails(); } + + // CamelCase class name -> snake_case variable, e.g. AlNanocrystals -> al_nanocrystals. function toSnake(name) { return name.replace(/([a-z0-9])([A-Z])/g, "$1_$2") .replace(/([A-Z]+)([A-Z][a-z])/g, "$1_$2").toLowerCase(); } - function copyText(text, btn) { - var done = function () { - var old = btn.textContent; btn.textContent = "Copied!"; btn.classList.add("copied"); - setTimeout(function () { btn.textContent = old; btn.classList.remove("copied"); }, 1100); - }; - if (navigator.clipboard && navigator.clipboard.writeText) { - navigator.clipboard.writeText(text).then(done).catch(function () { fallbackCopy(text, done); }); - } else { fallbackCopy(text, done); } - } - function fallbackCopy(text, done) { - var ta = document.createElement("textarea"); - ta.value = text; ta.style.position = "fixed"; ta.style.opacity = "0"; - document.body.appendChild(ta); ta.select(); - try { document.execCommand("copy"); done(); } catch (e) {} - ta.remove(); - } - - var header = el("div", "emdb-header"); - var tabsEl = el("div", "emdb-tabs"); - var body = el("div", "emdb-body"); - var listEl = el("div", "emdb-list"); - var detailsEl = el("div", "emdb-details"); - body.appendChild(listEl); body.appendChild(detailsEl); - root.appendChild(header); root.appendChild(tabsEl); root.appendChild(body); - function allItems() { - return (DATA.groups || []).reduce(function (a, g) { return a.concat(g.items); }, []); - } - function techniques() { return (DATA.groups || []).map(function (g) { return g.technique; }); } - function matchesSearch(it) { - if (!state.search) return true; - var blob = it.search || it.name.toLowerCase(); - return state.search.toLowerCase().split(/\s+/).every(function (t) { return blob.indexOf(t) !== -1; }); - } - function findItem(n) { return allItems().filter(function (i) { return i.name === n; })[0] || null; } - - function drawHeader() { - header.innerHTML = ""; - var top = el("div", "emdb-header-top"); - top.appendChild(el("div", "emdb-brand", - '◆ ' + esc(WHAT))); - top.appendChild(el("div", "emdb-count", DATA.n_total + " " + WHAT.toLowerCase())); - header.appendChild(top); - var search = el("input", "emdb-search"); - search.type = "text"; search.value = state.search; - search.placeholder = "Search " + WHAT.toLowerCase() + "…"; - search.addEventListener("input", function () { state.search = search.value; drawList(); }); - header.appendChild(search); - } - function drawTabs() { - tabsEl.innerHTML = ""; - var tabList = (typeof TABS !== "undefined" && TABS) ? TABS : techniques(); - ["All"].concat(tabList).forEach(function (tab) { - var b = el("button", "emdb-tab" + (state.tab === tab ? " active" : ""), esc(tab)); - b.addEventListener("click", function () { state.tab = tab; drawTabs(); drawList(); }); - tabsEl.appendChild(b); - }); - } - function drawList() { - listEl.innerHTML = ""; - var shown = 0; - // A dataset with several techniques is in several groups, so the All view - // lists it under the first one and skips it after that. - var drawn = {}; - (DATA.groups || []).forEach(function (g) { - if (state.tab !== "All" && g.technique !== state.tab) return; - var items = g.items.filter(matchesSearch).filter(function (it) { return !drawn[it.name]; }); - if (!items.length) return; - if (state.tab === "All") listEl.appendChild(el("div", "emdb-group-head", esc(g.technique))); - items.forEach(function (it) { - if (state.tab === "All") drawn[it.name] = true; - listEl.appendChild(drawRow(it)); shown++; - }); - }); - if (!shown) listEl.appendChild( - el("div", "emdb-empty", "No " + WHAT.toLowerCase() + " match.")); - if (!state.selected && allItems().length) state.selected = allItems()[0].name; - drawDetails(); - } - function drawRow(it) { - var row = el("div", "emdb-row" + (state.selected === it.name ? " selected" : "")); - row.appendChild(el("span", "emdb-glyph off", "•")); - row.appendChild(el("span", "emdb-name", esc(it.name))); - var meta = [it.size, it.shape].filter(Boolean).join(" · "); - row.appendChild(el("span", "emdb-meta", esc(meta))); - row.addEventListener("mouseenter", function () { state.hovered = it.name; drawDetails(); }); - row.addEventListener("click", function () { state.selected = it.name; drawList(); }); - return row; - } - function copyRow(shown, val) { - var row = el("div", "emdb-copy"); - row.appendChild(el("code", "emdb-code", esc(shown))); - var btn = el("button", "emdb-copy-btn", "Copy"); - btn.addEventListener("click", function () { copyText(val, btn); }); - row.appendChild(btn); - return row; - } - // Which version of an entry is being shown: "" is latest, and is all a - // dataset (or a family whose selection has gone away) ever has. - function currentVersion(it) { - var chosen = state.version[it.name]; - if (!chosen) return ""; - return (it.versions || []).some(function (v) { return v.version === chosen; }) ? chosen : ""; - } - // The link, pin and local file name of one version; an entry's own fields - // describe latest, so the same reads serve either. - function versionState(it, want) { - if (!want) return { url: it.url, checksum: it.latest_checksum, file: it.file }; - var row = (it.versions || []).filter(function (v) { return v.version === want; })[0] || {}; - return { url: row.url, checksum: row.checksum, file: versionedFile(it.file, want) }; - } - // Mirrors emdatabase.metadata.versioned_filename: w.pt -> w_260902.pt. - function versionedFile(file, version) { - return String(file || "").replace(/(\.[^.]*)$/, "_" + version + "$1"); - } - function versionSelect(it, version) { - var select = el("select", "emdb-version"); - select.title = "Which version to load or download"; - [["", "latest"]].concat((it.versions || []).map(function (v) { - return [v.version, v.version]; - })).forEach(function (choice) { - var option = document.createElement("option"); - option.value = choice[0]; - option.textContent = choice[1]; - option.selected = choice[0] === version; - select.appendChild(option); - }); - select.addEventListener("change", function () { - state.version[it.name] = select.value; - drawDetails(); - }); - return select; - } function drawDetails() { - var it = findItem(state.hovered || state.selected); + const item = allItems.find((it) => it.name === (state.hovered || state.selected)); + if (!item) { + detailsEl.innerHTML = `
Hover or select an entry.
`; + return; + } + const version = shownVersion(item, view); + const pin = versionState(item, version); // its link, checksum and saved file name detailsEl.innerHTML = ""; - if (!it) { detailsEl.appendChild(el("div", "emdb-details-empty", "Hover or select an entry.")); return; } - var version = currentVersion(it); - var pin = versionState(it, version); - var title = el("div", "emdb-d-title", esc(it.name)); - if (it.kind === "weights") title.appendChild(el("span", "emdb-kind", "weights")); - if ((it.versions || []).length) title.appendChild(versionSelect(it, version)); - detailsEl.appendChild(title); - detailsEl.appendChild(el("div", "emdb-d-sub", - esc([(it.technique || []).join(", "), it.size, it.shape] - .filter(Boolean).join(" · ")))); - if (it.description) detailsEl.appendChild(el("p", "emdb-d-desc", esc(it.description))); - var pairs = [["Detector", it.detector], ["Microscope", it.microscope], ["Voltage", it.voltage], - ["Tags", (it.tags || []).join(", ")], ["Authors", (it.authors || []).join(", ")], - ["License", it.license], ["DOI", it.doi], - ["Versions", (it.versions || []).map(function (v) { return v.version; }).join(", ")], - ["md5", it.kind === "weights" ? String(pin.checksum || "").replace(/^md5:/, "") : ""], - ["Model", it.model_class], ["Framework", it.model_framework], - ["quantem", it.model_quantem]]; - var meta = el("div", "emdb-d-meta"); - pairs.forEach(function (kv) { - if (!kv[1]) return; - var row = el("div", "emdb-kv"); - row.appendChild(el("span", "emdb-k", kv[0])); - row.appendChild(el("span", "emdb-v", esc(kv[1]))); - meta.appendChild(row); - }); - detailsEl.appendChild(meta); + drawHead(detailsEl, item, version, view); + if (item.description) detailsEl.appendChild(el("p", "emdb-d-desc", esc(item.description))); + const md5 = item.kind === "weights" ? (version ? pin.checksum : item.latest_checksum) : ""; + detailsEl.appendChild(drawMeta(item, [["md5", (md5 || "").replace(/^md5:/, "")]])); + detailsEl.appendChild(el("div", "emdb-load-label", "Load")); - var call = version ? '().download(version="' + version + '")' : "().download()"; - var snippet = it.kind === "weights" - ? "import torch\nfrom emdatabase import data\n\npath = data." + it.name - + call + "\ncheckpoint = torch.load(path, weights_only=True)" - : toSnake(it.name) + " = emdatabase.data." + it.name + "()"; - detailsEl.appendChild(copyRow(snippet, snippet)); - if (pin.url) { - var wrap = el("div", "emdb-dl-link"); - var a = document.createElement("a"); - a.href = pin.url; a.target = "_blank"; a.rel = "noopener"; - a.className = "emdb-dl-anchor"; a.textContent = "⤓ Download " + pin.file; - wrap.appendChild(a); - detailsEl.appendChild(wrap); - } + const call = version ? `().download(version="${version}")` : "().download()"; + detailsEl.appendChild(copyRow(item.kind === "weights" + ? `import torch\nfrom emdatabase import data\n\npath = data.${item.name}${call}\n` + + "checkpoint = torch.load(path, weights_only=True)" + : `${toSnake(item.name)} = emdatabase.data.${item.name}()`)); + const link = el("a", "emdb-dl-anchor"); + link.href = pin.url; + link.target = "_blank"; + link.rel = "noopener"; + link.textContent = `⤓ Download ${pin.file}`; + const wrap = el("div", "emdb-dl-link"); + wrap.appendChild(link); + detailsEl.appendChild(wrap); } - drawHeader(); drawTabs(); drawList(); + fillTabs(tabsEl, TABS, state, drawList); + drawList(); })(); """ -# --------------------------------------------------------------------------- -# Shared "app" chrome: a Catppuccin-Mocha shell that makes the whole docs site -# look like emdatabase.browse(). Every generated page (landing / all-data / -# weights / add-dataset) is a self-contained file: the widget CSS is inlined, -# the palette and top-nav live in _APP_CSS, and the catalogue JSON is baked in -# at build time so search and the list work with no backend and no external -# requests. -# --------------------------------------------------------------------------- - # Palette + top-nav + hero, keyed to the same Catppuccin tokens browser.css # defines on .emdb (mirrored here on :root so the nav and hero get them too). _APP_CSS = """ @@ -637,7 +107,7 @@ def generate_html_table(datasets_by_technique): --emdb-surface0: #313244; --emdb-surface1: #45475a; --emdb-surface2: #585b70; --emdb-overlay: #2a2a3c; --emdb-text: #cdd6f4; --emdb-subtext: #a6adc8; --emdb-muted: #7f849c; --emdb-blue: #89b4fa; --emdb-mauve: #cba6f7; - --emdb-green: #a6e3a1; --emdb-red: #f38ba8; --emdb-yellow: #f9e2af; + --emdb-green: #a6e3a1; --emdb-red: #f38ba8; --emdb-font: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, sans-serif; --emdb-mono: ui-monospace, "SF Mono", "JetBrains Mono", Menlo, monospace; } @@ -662,10 +132,6 @@ def generate_html_table(datasets_by_technique): display: flex; align-items: center; gap: 8px; color: var(--emdb-text); } .app-brand:hover { text-decoration: none; } -.app-brand .diamond { - background: linear-gradient(135deg, var(--emdb-blue), var(--emdb-mauve)); - -webkit-background-clip: text; background-clip: text; -webkit-text-fill-color: transparent; -} .app-navlink { font-size: 14px; font-weight: 600; color: var(--emdb-subtext); padding: 7px 15px; border-radius: 999px; border: 1px solid transparent; @@ -678,13 +144,15 @@ def generate_html_table(datasets_by_technique): .app-main { max-width: 100%; margin: 0; padding: 20px 40px 60px; } -/* Hero --------------------------------------------------------------- */ +/* Hero: large on the landing page, smaller (.page) on the others ------ */ .app-hero { text-align: center; padding: 30px 0 14px; } .app-hero h1 { margin: 0; font-size: 40px; font-weight: 800; letter-spacing: -0.5px; line-height: 1.1; background: linear-gradient(135deg, #89b4fa, #cba6f7); -webkit-background-clip: text; background-clip: text; -webkit-text-fill-color: transparent; } +.app-hero.page { padding: 18px 0 4px; } +.app-hero.page h1 { font-size: 30px; } .app-hero p { margin: 12px auto 0; max-width: 720px; color: var(--emdb-subtext); font-size: 16px; } .app-hero code { font-family: var(--emdb-mono); background: var(--emdb-crust); @@ -696,8 +164,7 @@ def generate_html_table(datasets_by_technique): # .emdb at 75% for the notebook) and styles the static download link. _BROWSER_OVERRIDES = """ .emdb { max-width: 100%; margin: 8px 0 0; font-size: 14px; } -.emdb-diamond { display: none; } -.emdb-body { height: 600px; } +.emdb-body { height: 620px; } .emdb-list { min-width: 360px; max-width: 50%; } .emdb-dl-link { margin-top: 14px; } .emdb-dl-anchor { color: var(--emdb-blue); text-decoration: none; font-size: 14px; font-weight: 600; } @@ -716,8 +183,8 @@ def generate_html_table(datasets_by_technique): .emdb-kv { font-size: 13.5px; } .emdb-k { flex-basis: 92px; } .emdb-code { font-size: 13px; } -.emdb-load-label, .emdb-d-status { font-size: 12px; } -.emdb-copy-btn, .emdb-dl { font-size: 13px; } +.emdb-load-label { font-size: 12px; } +.emdb-copy-btn { font-size: 13px; } /* Keep the metadata a tidy two-column block instead of sprawling edge to edge */ .emdb-d-meta { grid-template-columns: repeat(2, minmax(0, 1fr)); max-width: 820px; gap: 6px 30px; } """ @@ -730,8 +197,6 @@ def generate_html_table(datasets_by_technique): color: var(--emdb-subtext); margin: 34px 0 12px; padding-top: 18px; border-top: 1px solid var(--emdb-surface0); } -.explainer ul { padding-left: 20px; margin: 0; } -.explainer li { margin: 8px 0; } .explainer code { font-family: var(--emdb-mono); font-size: 13.5px; color: var(--emdb-text); background: var(--emdb-crust); border: 1px solid var(--emdb-surface0); @@ -753,23 +218,9 @@ def generate_html_table(datasets_by_technique): .btn-primary:hover { filter: brightness(1.06); text-decoration: none; } """ - -def _esc(value) -> str: - """Minimal HTML escaping for text baked into a page at build time.""" - return ( - str(value) - .replace("&", "&") - .replace("<", "<") - .replace(">", ">") - .replace('"', """) - .replace("'", "'") - ) - - # Top-nav destinations. Examples and API are Sphinx-generated (sphinx-gallery + -# autodoc); All Data and Add Dataset are the generated app pages. All the -# generated pages sit at the site root, so these relative links resolve the -# same from each of them. +# autodoc); the rest are the generated app pages. Every page sits at the site +# root, so these relative links resolve the same from each of them. _NAV_LINKS = ( ("Examples", "examples/index.html"), ("API", "reference/index.html"), @@ -779,130 +230,103 @@ def _esc(value) -> str: ) -def _top_nav(active: str = "") -> str: - items = "".join( - '{name}'.format( - cls=" active" if name == active else "", url=url, name=_esc(name) - ) - for name, url in _NAV_LINKS - ) - return ( - '" - ) - - -def _load_css() -> str: - return (resources.files("emdatabase") / "static" / "browser.css").read_text(encoding="utf-8") - - def _app_page( title: str, body: str, active: str = "", extra_css: str = "", scripts: str = "" ) -> str: """Wrap page ``body`` in the self-contained Catppuccin app shell. Built by concatenation (not ``str.format``/``%``) so CSS/JS braces need no - escaping. Every page is self-contained: the CSS and any ``scripts`` are - inlined, and none of them loads an external resource. + escaping. """ + nav = '\n" + + body + + "\n" + + scripts + + "\n\n" ) -def _catalogue_payload(kind: str = "dataset"): - """``(payload, tabs)`` - the baked catalogue and the ordered tab list. +def _catalogue_payload(kind: str): + """``(payload, tabs)`` - the catalogue to bake into a page, and its tab list. Tabs are every acquisition technique, so one with no dataset in it yet - still shows, plus anything else present, in the browser's group order. For - the weights page there is one group, so the tabs are whatever is there. + still shows, plus anything else present, in the browser's group order; the + weights page has its one group. What is on the build machine's disk is no + concern of a reader's, so it is left out, and each version is given the + name it is saved under. """ from emdatabase import catalogue - payload = catalogue.catalogue(kind=kind) - present = [g["technique"] for g in payload.get("groups", [])] + cat = catalogue.catalogue(kind=kind) + for group in cat["groups"]: + for item in group["items"]: + item.update(downloaded=False, location=None, path="", user_path="") + for row in item["versions"]: + row.update(downloaded=False, location=None, path="") + row["file"] = versioned_filename(item["file"], row["version"]) + payload = {"groups": cat["groups"], "n_total": cat["n_total"]} + present = [group["technique"] for group in cat["groups"]] if kind == "weights": return payload, present return payload, catalogue.ordered_groups([*acquisition_techniques(), *present]) -def _browser_script(payload, tabs, label: str = "Datasets") -> str: - """The " - ) - - -def generate_browser_html() -> str: - """Self-contained, widget-styled dataset browser (no nav chrome). - - Kept for backward compatibility (it can be embedded in an `` \ No newline at end of file diff --git a/docs/source/index.rst b/docs/source/index.rst index 2229dc3..2a3a911 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -18,8 +18,4 @@ lines of Python from your analysis. .. note:: - This landing page is rendered as a self-contained, widget-styled app during - the build (see ``build-finished`` in ``conf.py``); this reStructuredText is a - fallback that only shows if that generation step fails. Browse the datasets on - the :doc:`All Data ` page, or see :doc:`Add Dataset ` - for how to contribute one. + The build replaces this page with a generated one (see ``conf.py``). diff --git a/docs/source/intro.rst b/docs/source/intro.rst deleted file mode 100644 index a79d0ff..0000000 --- a/docs/source/intro.rst +++ /dev/null @@ -1,54 +0,0 @@ - -EM-Database -=========== - -A curated, citable collection of electron microscopy datasets — a couple of lines -of Python from your analysis. Browse below, then copy the snippet to load one. - -.. raw:: html - - - - -This package works as an accumulator for electron microscopy data. The idea is to -provide a simple API to programmatically access EM data stored in a variety of different -places. - - - - -Learning resources -================== - -.. grid:: 1 3 3 3 - :gutter: 2 - - .. grid-item-card:: - :link: examples/index - :link-type: doc - - :octicon:`zap;2em;sd-text-info` Examples - ^^^ - - Gallery of short examples illustrating simple tasks that can be performed with EM-Database. - - .. grid-item-card:: - :link: datasets - :link-type: doc - - :octicon:`database;2em;sd-text-info` Datasets - ^^^ - - Browse available electron microscopy datasets and learn how to access them. - - .. grid-item-card:: - :link: reference/index - :link-type: doc - - :octicon:`code-square;2em;sd-text-info` API - ^^^ - - Documentation of the Application Programming Interface (API), - which describes how EM-Database functions work and which parameters can be used. diff --git a/docs/source/weights.rst b/docs/source/weights.rst index 2be0f8b..db4e709 100644 --- a/docs/source/weights.rst +++ b/docs/source/weights.rst @@ -10,7 +10,4 @@ and fetched with ``download(version="260902")``. .. note:: - This page is rendered as a self-contained, widget-styled browser during the - build (see ``build-finished`` in ``conf.py``); this text is a fallback that - only shows if that generation step fails. See :doc:`Contributing a Dataset - ` for what a weights entry has to declare. + The build replaces this page with a generated one (see ``conf.py``). diff --git a/emdatabase/static/browser.js b/emdatabase/static/browser.js index 48adda3..e38dd7e 100644 --- a/emdatabase/static/browser.js +++ b/emdatabase/static/browser.js @@ -42,41 +42,14 @@ function render({ model, el: root }) { } function drawTabs() { - tabsEl.innerHTML = ""; - for (const tab of ["All", ...model.get("groups").map((g) => g.technique)]) { - const btn = el("button", "emdb-tab" + (state.tab === tab ? " active" : ""), esc(tab)); - btn.addEventListener("click", () => { state.tab = tab; drawTabs(); drawList(); }); - tabsEl.appendChild(btn); - } - } - - // `item.search` is a lowercased blob of every field (name, description, - // detector, microscope, tags, authors + affiliations, license, …), so a - // query like "Carter Francis" matches on author, not just the name. - // emdatabase.search() matches this same blob by this same rule; matching - // stays here rather than in the kernel so typing never waits on a round trip. - function matchesSearch(item) { - return state.search.toLowerCase().split(/\s+/).every((term) => item.search.includes(term)); + fillTabs(tabsEl, model.get("groups").map((g) => g.technique), state, drawList); } function drawList() { // A row is marked whichever version of it is running. const active = new Set([...activeLabels(view)].map((label) => label.split("@")[0])); - listEl.innerHTML = ""; - // A dataset with several techniques is in several groups, so the All view - // lists it under the first one and skips it after that. - const drawn = new Set(); - for (const group of model.get("groups")) { - if (state.tab !== "All" && group.technique !== state.tab) continue; - const items = group.items.filter((it) => matchesSearch(it) && !drawn.has(it.name)); - if (!items.length) continue; - if (state.tab === "All") { - listEl.appendChild(el("div", "emdb-group-head", esc(group.technique))); - for (const item of items) drawn.add(item.name); - } - for (const item of items) listEl.appendChild(drawRow(item, active.has(item.name))); - } - if (!listEl.children.length) listEl.appendChild(el("div", "emdb-empty", "No datasets match.")); + fillList(listEl, model.get("groups"), state, (item) => drawRow(item, active.has(item.name)), + "No datasets match."); state.selected ??= allItems()[0]?.name; drawDetails(); } diff --git a/emdatabase/static/common.js b/emdatabase/static/common.js index 4371566..a71e88b 100644 --- a/emdatabase/static/common.js +++ b/emdatabase/static/common.js @@ -1,8 +1,8 @@ -// common.js - what browser.js and card.js share. +// common.js - what browser.js, card.js and the docs site's browser share. // // anywidget loads each widget's `_esm` as a single module and there is no -// bundler, so widget.py puts this file in front of each widget's own file -// rather than having them import it. +// bundler, so widget.py (and docs/source/_build_docs.py) put this file in front +// of each one rather than having them import it. const MB = 1e6; @@ -159,8 +159,60 @@ function drawToasts(stack, view) { } } +// --- tabs and list ----------------------------------------------------- + +// "All", then a tab per entry of `tabs`; picking one sets `state.tab` and +// calls `onPick`. +function fillTabs(tabsEl, tabs, state, onPick) { + tabsEl.innerHTML = ""; + for (const tab of ["All", ...tabs]) { + const btn = el("button", "emdb-tab" + (state.tab === tab ? " active" : ""), esc(tab)); + btn.addEventListener("click", () => { + state.tab = tab; + fillTabs(tabsEl, tabs, state, onPick); + onPick(); + }); + tabsEl.appendChild(btn); + } +} + +// `drawRow(item)` for each entry of `groups` in `state.tab` that matches +// `state.search`, under a heading per group in the All view, or `empty`. +// +// `item.search` is a lowercased blob of every field (name, description, +// detector, microscope, tags, authors + affiliations, license, …), so a query +// like "Carter Francis" matches on author, not just the name. +// emdatabase.search() matches this same blob by this same rule; matching stays +// here rather than in the kernel so typing never waits on a round trip. +function fillList(listEl, groups, state, drawRow, empty) { + listEl.innerHTML = ""; + const terms = state.search.toLowerCase().split(/\s+/); + // A dataset with several techniques is in several groups, so the All view + // lists it under the first one and skips it after that. + const drawn = new Set(); + for (const group of groups) { + if (state.tab !== "All" && group.technique !== state.tab) continue; + const items = group.items.filter( + (it) => !drawn.has(it.name) && terms.every((term) => it.search.includes(term))); + if (!items.length) continue; + if (state.tab === "All") { + listEl.appendChild(el("div", "emdb-group-head", esc(group.technique))); + for (const item of items) drawn.add(item.name); + } + for (const item of items) listEl.appendChild(drawRow(item)); + } + if (!listEl.children.length) listEl.appendChild(el("div", "emdb-empty", empty)); +} + // --- one catalogue row ------------------------------------------------- +// Which version of `item` is shown: "" is latest, and is all a dataset (or a +// family whose chosen version has gone away) ever has. +function shownVersion(item, view) { + const chosen = view.versions[item.name]; + return item.versions.some((v) => v.version === chosen) ? chosen : ""; +} + // The download state of one version: an item's own fields describe latest, so // the same reads work for the item or for one of its `versions`. function versionState(item, version) { @@ -189,21 +241,15 @@ function copyRow(text, variant) { return row; } -// Draw `item` into `parent`: what the browser's details panel and a dataset's -// card both show. A weights family gets a version picker - `latest`, then the -// dated snapshots, ● on those on disk - and the status, the path and the load -// snippet all follow it. `toasts`, if given, goes under the status line. -function drawEntry(parent, item, view, toasts) { - const chosen = view.versions[item.name]; - const version = item.versions.some((v) => v.version === chosen) ? chosen : ""; - const where = versionState(item, version); - parent.innerHTML = ""; - +// The title and the line under it. A weights family's title has a version +// picker - `latest`, then the dated snapshots, ● on those on disk - which +// records the choice in `view.versions` and redraws. +function drawHead(parent, item, version, view) { const title = el("div", "emdb-d-title", esc(item.name)); if (item.kind === "weights") title.appendChild(el("span", "emdb-kind", "weights")); if (item.versions.length) { const select = el("select", "emdb-version"); - select.title = "Which version to download, delete or load"; + select.title = "Which version to show"; for (const value of ["", ...item.versions.map((v) => v.version)]) { const option = document.createElement("option"); option.value = value; @@ -217,9 +263,41 @@ function drawEntry(parent, item, view, toasts) { }); title.appendChild(select); } - parent.appendChild(title); const sub = [item.technique.join(", "), item.size].filter(Boolean).join(" · "); - parent.appendChild(el("div", "emdb-d-sub", esc(sub))); + parent.append(title, el("div", "emdb-d-sub", esc(sub))); +} + +// The metadata as label/value rows, then any `extra` pairs; empty values are +// left out. +function drawMeta(item, extra = []) { + const meta = el("div", "emdb-d-meta"); + const pairs = [ + ["Detector", item.detector], ["Microscope", item.microscope], ["Voltage", item.voltage], + ["Tags", item.tags.join(", ")], ["Authors", item.authors.join(", ")], + ["License", item.license], ["File", item.file], ["DOI", item.doi], + ["Versions", item.versions.map((v) => v.version).join(", ")], + ["Model", item.model_class], ["Framework", item.model_framework], + ["quantem", item.model_quantem], + ...extra, + ]; + for (const [key, value] of pairs) { + if (!value) continue; + const kv = el("div", "emdb-kv"); + kv.appendChild(el("span", "emdb-k", key)); + kv.appendChild(el("span", "emdb-v", esc(value))); + meta.appendChild(kv); + } + return meta; +} + +// Draw `item` into `parent`: what the browser's details panel and a dataset's +// card both show. The status, the path and the load snippet follow the version +// picked. `toasts`, if given, goes under the status line. +function drawEntry(parent, item, view, toasts) { + const version = shownVersion(item, view); + const where = versionState(item, version); + parent.innerHTML = ""; + drawHead(parent, item, version, view); const status = el("div", "emdb-d-status"); const deleteButton = (text, tooltip) => { @@ -257,24 +335,7 @@ function drawEntry(parent, item, view, toasts) { + (version ? `version="${version}"` : "") + ")")); if (where.path) main.appendChild(copyRow(where.path, "path")); - const meta = el("div", "emdb-d-meta"); - const pairs = [ - ["Detector", item.detector], ["Microscope", item.microscope], ["Voltage", item.voltage], - ["Tags", item.tags.join(", ")], ["Authors", item.authors.join(", ")], - ["License", item.license], ["File", item.file], ["DOI", item.doi], - ["Versions", item.versions.map((v) => v.version).join(", ")], - ["Model", item.model_class], ["Framework", item.model_framework], - ["quantem", item.model_quantem], - ]; - for (const [key, value] of pairs) { - if (!value) continue; - const kv = el("div", "emdb-kv"); - kv.appendChild(el("span", "emdb-k", key)); - kv.appendChild(el("span", "emdb-v", esc(value))); - meta.appendChild(kv); - } - const cols = el("div", "emdb-d-cols"); - cols.append(main, meta); + cols.append(main, drawMeta(item)); parent.appendChild(cols); } diff --git a/emdatabase/tests/test_issue_route.py b/emdatabase/tests/test_issue_route.py index 1072721..aa0b65f 100644 --- a/emdatabase/tests/test_issue_route.py +++ b/emdatabase/tests/test_issue_route.py @@ -5,10 +5,9 @@ and its keys are compared against ``new_dataset.FIELD_ORDER``. One test runs the same entry through both routes and compares the bytes. -The docs site's datasets table is built from the same YAML by the same module, -so its tab grouping is checked here too, along with the Add Dataset page, which -is now an explainer pointing at the issue form and the CLI rather than a form of -its own. +The docs site's Add Dataset page, which points at the issue form and the CLI, +is checked here too, along with the rule that no generated page loads anything +from outside. The issue script lives in ``.github/scripts`` rather than in the package and is loaded from its path. Nothing here touches the network: the calls that would are @@ -446,40 +445,7 @@ def test_add_dataset_page_points_at_the_issue_form_and_the_cli(build_docs): def test_generated_pages_load_nothing_from_outside(build_docs): """Every page is self-contained; the only external host left is a link target.""" - for generate in ( - build_docs.generate_add_dataset_html, - build_docs.generate_landing_html, - build_docs.generate_all_data_html, - build_docs.generate_weights_html, - ): + for generate in build_docs.PAGES.values(): html = generate() assert "