Skip to content

feat(18): adopt the shared CLI helpers in the rdkit Jobs - #24

Merged
tdudgeon merged 1 commit into
mainfrom
feat/18-adopt-shared-cli-helpers
Aug 11, 2026
Merged

feat(18): adopt the shared CLI helpers in the rdkit Jobs#24
tdudgeon merged 1 commit into
mainfrom
feat/18-adopt-shared-cli-helpers

Conversation

@tdudgeon

Copy link
Copy Markdown
Collaborator

Sixth consumer migration for InformaticsMatters/squonk2-jobs#18, and the first
of two PRs for this repo.

Summary

Replaces the hand-typed "Input/output options" block in seven scripts with
rdkit_utils.add_common_molecule_io_args(), and the interval reporting idiom
with dm_job_utilities' add_reporting_args() / ProgressReporter:

rdkit_props.py, rdkit_dedup.py, sa_score.py, cluster_butina.py,
enumerate.py, le_conformers.py, im_mordred/descriptor_generator.py

Net −109/+45 lines. Dockerfile-prep and Dockerfile-mordred bumped to
im-data-manager-job-utilities>=1.4.0 / im-rdkit-utilities>=1.1.2.

No manifest changes. --input and --output are retained as aliases by the
helper, so every existing command block keeps working — this is exactly what the
alias design in
squonk2-jobs#17
was for. The sweep to canonical --infile/--outfile is deferred to before the
release that drops the aliases.

⚠️ screen.py is excluded — a real collision

It uses -d for --descriptor, which collides with the helper's -d for
--delimiter:

argparse.ArgumentError: argument -d/--delimiter: conflicting option string: -d

screen.py is otherwise a strong adopter (9/11 options), so this is a genuine
loss. Resolving it needs a decision:

  1. Drop -d from --descriptor. The manifest passes --descriptor
    long-form, so no manifest change — but it removes a public short flag.
  2. Give the helper a way to omit -d, e.g. a short_flags knob. Costs an
    im-rdkit-utilities release and adds API surface for one caller.

Left out rather than decided unilaterally.

Other judgement calls

  • cluster_butina.py takes the I/O group but not add_reporting_args()
    it has no progress loop and never had --interval, so adding one would be
    noise.
  • sa_score.py, le_conformers.py and enumerate.py gain --omit-fields
    and/or --write-header
    they did not previously have, as members of the
    shared group. Unused and optional; the accepted cost of a common group.
  • --mol-column now defaults to None so SmilesReader infers it. With
    --id-column 0 the old hardcoded 0 pointed the molecule and ID columns at
    the same field. No manifest passes --mol-column, so the default path is
    unchanged.

Verification

Output parity against main, same inputs and arguments:

Script Result
rdkit_props.py byte-identical
rdkit_dedup.py byte-identical
sa_score.py byte-identical
cluster_butina.py byte-identical
enumerate.py byte-identical
le_conformers.py byte-identical

im_mordred/descriptor_generator.py could not be run locally (mordred not
installed — it fails identically on main). Its parsed namespace is
identical old vs new, and the same file lineage was validated end-to-end
with jote in
squonk2-desc-mordred#3.

Parsed-namespace comparison, old vs new, for all seven. The only differences
are the four intended ones:

  • id_column: '1'1 (the str_or_int type)
  • mol_column: 0None (reader inference, above)
  • outfileoutput (dest rename; call sites updated)
  • new omit_fields / write_header keys where the group adds them

Building every parser also proves there are no other short-flag collisions
screen.py was the only one.

jote was not run — these Jobs need the informaticsmatters/vs-prep image
rebuilt. Worth a CI run before merge.

Follow-up PR

utils.py is untouched here. Removing it needs the two functions #18 flags as
blockers resolved, and this repo is the only consumer that actually hits them:

  • round_to_significant_number — 5 uses, all in dmpk/pk_tmax_cmax_sim.py.
    Replacing it with sigfig.round changes numeric output, so it needs its
    own validation and shouldn't ride along with a CLI refactor.
  • get_path_from_digest — 1 use in assemble_conformers.py.

🤖 Generated with Claude Code

Replaces the hand-typed "Input/output options" block in seven scripts with
rdkit_utils.add_common_molecule_io_args(), and the interval reporting idiom
with dm_job_utilities' add_reporting_args() / ProgressReporter.

No manifest changes: --input and --output are retained as aliases by the
helper, so every existing command block keeps working.

screen.py is deliberately left alone. It uses -d for --descriptor, which
collides with the helper's -d for --delimiter:

  argparse.ArgumentError: argument -d/--delimiter: conflicting option
  string: -d

Resolving that means either dropping a public short flag from --descriptor or
giving the helper a way to omit -d, and neither should be decided inside this
change.

cluster_butina.py takes the I/O group but not add_reporting_args(): it has no
progress loop and never had --interval, so adding one would be noise.

--mol-column now defaults to None so SmilesReader infers it. With
--id-column 0 the old hardcoded 0 pointed the molecule and ID columns at the
same field; no manifest passes --mol-column, so the default path is unchanged.

utils.py is untouched here. Removing it needs round_to_significant_number
(5 uses in dmpk/pk_tmax_cmax_sim.py) replaced with sigfig.round, which
changes numeric output, and get_path_from_digest (1 use in
assemble_conformers.py) resolved. That follows separately.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tdudgeon
tdudgeon merged commit 6003c0e into main Aug 11, 2026
12 checks passed
@tdudgeon
tdudgeon deleted the feat/18-adopt-shared-cli-helpers branch August 11, 2026 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant