Skip an article with no usable PMCID instead of failing its whole batch by adelavega · Pull Request #61 · neuroquery/pubget

adelavega · 2026-09-25T01:08:01Z

One unidentifiable record currently discards every sibling article in its batch.

_extract_from_articleset calls _utils.get_pmcid(article) inside its per-batch
loop. That raises ValueError for records carrying no resolvable identifier, and
nothing catches it, so the exception propagates out of the loop. Batches are
typically hundreds of articles, so a single bad record throws away every sibling
that had already downloaded successfully — and the only symptom is one exception
line, with the downloads silently gone.

Observed on a real retrieval of 413 PMCIDs, which produced zero article
directories because a handful of records could not be identified.

This catches the ValueError, logs a warning naming the batch file, and
continues to the next article.

Adds a regression test covering an unidentifiable article alongside a pmc-typed
and a pmcid-typed one, asserting the two resolvable articles are still
extracted.

The five pre-existing failures in tests/test_articles.py are unrelated to this
change and reproduce identically on a clean checkout of main.

🤖 Generated with Claude Code

get_pmcid raises ValueError for records carrying no resolvable identifier, and _extract_from_articleset let that propagate out of its per-batch loop. Batches are typically hundreds of articles, so a single unidentifiable record discarded every sibling that had downloaded successfully -- and the only symptom was one exception line, with the downloads silently gone. Observed on a real retrieval of 413 PMCIDs, which produced zero article directories because a handful of records could not be identified. Catch the ValueError, warn with the batch name, and continue. Adds a regression test covering an unidentifiable article alongside a pmc-typed and a pmcid-typed one; the two resolvable articles are still extracted. The five pre-existing failures in tests/test_articles.py are unrelated and reproduce identically on a clean checkout. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

adelavega · 2026-09-25T01:44:44Z

Failure looks unrelated

Both siblings resolved through ../ paths, so the environment could only be built in one directory on one machine -- the single largest obstacle to anyone reproducing this from a clone. autonima 440de05b77a1ce00ca05cc9b5c73755ac8987b11 (neurostuff/autonima) ace d64291e06655d60049e046baeb670f8ffb5db360 (neurosynth/ACE) autonima is pinned to where master stood LOCALLY when the evaluation was run (2026-09-08), not to origin/master. The eight commits between them change how the model is called -- model-parameter passthrough, model_params read from config -- and the canonical runs finished 2026-08-27. One caveat that belongs in Methods rather than here: Supplementary S5's cost figures were measured 2026-09-04 and so come from fef53ac, which is after this pin. No single commit produced every number in the paper, and execution_manifest.json records no version, so the mapping cannot be recovered from the artifacts. ACE's no_miss branch was already merged upstream, so d64291e (origin/master) carries it. ACE supplies scraped HTML as a retrieval source (articles/ace_outputs); the manuscript covers that as "user-provided article files". pubget is deliberately left on its local path. The retrieval used a fix that exists on no public commit -- "Skip an article with no usable PMCID instead of failing its whole batch" -- now open as neuroquery/pubget#61. Pin the merge commit, or a release, once it lands. NOT VERIFIED: pixi cannot solve this manifest from a worktree, and the pins now require network access to clone. Run `pixi install` in the main checkout to confirm, together with the earlier removal of the editable self-install. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Pinning autonima and ACE to public commits made the default environment reproducible but broke the development loop: edits to ../autonima no longer reached the environment. Two pixi environments instead, sharing the conda dependencies and nimare: pixi run <cmd> default -- feature "pinned", the public revisions pixi run -e dev <cmd> development -- feature "dev", editable ../ checkouts The siblings moved out of the default pypi-dependencies into the two features, so each environment carries exactly one definition of each and they cannot conflict. reproduce.sh uses the default, which is what a reproduction should build against. paper/README.md said pinning "must be fixed before the repository is frozen". Two thirds of that is now done, so it records what remains instead: pubget is still resolved through ../pubget in both environments because the retrieval used a fix that exists on no public commit (neuroquery/pubget#61), and a clone cannot build either environment until that merges. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

jdkent

LGTM!

perhaps we will want better provenance on what articles fail besides a printed log string, but that's a separate item of work.

neuroquery/pubget#61 merged as 236b762, so the last dependency resolving through a ../ path is gone. It carries the fix the retrieval depended on: before it, one record with no usable PMCID discarded every sibling in its batch -- observed on a 413-PMCID retrieval that produced zero article directories. Pinned to the merge commit rather than a release because pubget has cut none since. All three siblings now resolve from public git: autonima v0.1.0, ACE d64291e, pubget 236b762. paper/README.md no longer warns that a clone cannot build the environment, because it can. Verified: pixi install resolves all three and manuscript_numbers runs clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

pubget had cut no release carrying the fix the retrieval depended on, so it was pinned to the merge commit of neuroquery/pubget#61. v0.0.9 is now cut and contains it, so the pin becomes a citable version. Behaviourally identical to the commit it replaces: the only change between 236b762 and v0.0.9 is a mypy annotation fix (#62) and the version bump. That fix was needed to get main green before tagging -- _extract_word_counts returned np.concatenate(...) under a Sequence[int] annotation, which numpy >= 2.1 stubs reject, so the py313-latest-deps leg of pubget's matrix had been failing. All three siblings are now pinned and no ../ path remains in the pinned environment, so it builds from a clone: autonima v0.1.0 ace d64291e pubget v0.0.9 Verified: pixi install solves and installs; the environment reports autonima 0.1.0, pubget 0.0.9, nimare 0.2.1; pubget carries the ValueError guard from #61; paper/manuscript_numbers.py runs clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

adelavega requested review from jdkent and jeromedockes September 25, 2026 01:33

jdkent approved these changes Sep 25, 2026

View reviewed changes

adelavega merged commit 236b762 into main Sep 25, 2026
7 of 8 checks passed

Navigation Menu

Repository navigation

Skip an article with no usable PMCID instead of failing its whole batch - #61

Skip an article with no usable PMCID instead of failing its whole batch#61
adelavega merged 1 commit into
mainfrom
fix/skip-unidentifiable-article

adelavega commented Sep 25, 2026

Uh oh!

adelavega commented Sep 25, 2026

Uh oh!

jdkent left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

adelavega commented Sep 25, 2026

Uh oh!

adelavega commented Sep 25, 2026

Uh oh!

jdkent left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants