fix(deps): make pyex installable as a bare dependency#139
Open
ivarvong wants to merge 3 commits into
Open
Conversation
pyex referenced three deps in lib/ that a fresh consumer wouldn't have, so
`{:pyex, "~> x"}` failed to compile for anyone — it only worked in-repo because
the deps were present transitively. Found while composing pyex with another
library; it's also a release blocker.
- decimal: used pervasively in the numeric tower but never declared. Now a
real runtime dependency (it isn't optional — it's core).
- postgrex (the `sql` backend): lib/pyex/stdlib/sql.ex pattern-matches
%Postgrex.Error{} structs — a hard compile-time requirement. The module is
now wrapped in `if Code.ensure_loaded?(Postgrex)`.
- explorer (the `pandas` backend): scattered Explorer.Series/DataFrame calls,
type specs, and struct patterns across core modules. Fixed idiomatically:
`@compile {:no_warn_undefined, ...}` for the calls, `term()` for the specs,
`is_struct(x, Explorer.Series)` guards (a runtime atom check, no compile-time
struct) for the patterns, and the producer module wrapped in
`Code.ensure_loaded?(Explorer)`.
postgrex/explorer stay `optional: true`: `import sql`/`import pandas` now raise
a clean Python ImportError when the backend isn't installed (Pyex.Stdlib.fetch/1
and module_names/0 degrade gracefully), so the core library carries no heavy
native deps.
Proven by `scripts/consumer_smoke.sh` + a `consumer-smoke` CI job: a throwaway
project depending on pyex and nothing else compiles and runs, with the optional
features degrading to ImportError — the regression class pyex's own build
(where the optional deps ARE present) cannot catch.
Full suite (6185) + Dialyzer green with the deps present.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019NokzcR7BiAigPgC78zpk9
Capture why pandas/sql are optional and how to add another optional backend, citing the idiomatic shapes (the same ones :explorer uses for its own optional :nx — @compile {:no_warn_undefined} + is_struct guards), with the consumer-smoke CI job as the regression guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019NokzcR7BiAigPgC78zpk9
A path dep ships the whole working tree, so it can't catch a compile-time file (an @external_resource, a data dir) left out of `package/0`'s `:files`. Verified the gap exists in principle, then closed it: consumer_smoke.sh now runs `mix hex.build`, unpacks the package, and compiles *that* as a bare dependency — the true "installs from hex" gate. Still asserts core runs and pandas/sql degrade to ImportError with no optional backends present. (The package was already complete — 145 files, incl. the spreadsheet.py @external_resource — so this is the guard, not a fix.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019NokzcR7BiAigPgC78zpk9
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
pyex referenced three deps in
lib/that a fresh consumer wouldn't have, so{:pyex, "~> x"}failed to compile for anyone. It only worked in-repo because the deps were present transitively. Found while composing pyex with another library — and it's a release blocker (#60 ships something that doesn't install).lib/used itdecimalpostgrex(sql)sql.expattern-matches%Postgrex.Error{}— a hard compile-time struct requirementif Code.ensure_loaded?(Postgrex)explorer(pandas)Explorer.Series/DataFramecalls +.t()specs + struct patterns across core modules@compile {:no_warn_undefined, …}for calls,term()for specs,is_struct(x, Explorer.Series)guards for patterns (a runtime atom check — no compile-time struct), producer module wrapped inCode.ensure_loaded?(Explorer)postgrex/explorerstayoptional: true, so the core library carries no heavy native deps.import sql/import pandasnow raise a clean PythonImportErrorwhen the backend isn't installed (Pyex.Stdlib.fetch/1andmodule_names/0degrade gracefully) instead of crashing.The idiomatic patterns (for the record)
decimal).if Code.ensure_loaded?(Dep)around the module (struct expansion is a hard compile-time requirement that nothing else can defer).@compile {:no_warn_undefined, [Dep.Mod]}.case/head →is_struct(x, Dep.Struct)guard (takes the module as a runtime atom; compiles when absent).Code.ensure_loaded?/function_exported?at the dispatch boundary.Proof it works — and stays working
scripts/consumer_smoke.sh+ a newconsumer-smokeCI job: a throwaway project depending on pyex and nothing else compiles and runs —It asserts
Explorer/Postgrexare genuinely absent, core runs, andimport pandas/import sql→%Pyex.Error{kind: :import}. This is the regression class pyex's own build (where the optional deps are present) can't catch.Gate
Full suite (6185) + Dialyzer green with the deps present; consumer-smoke green without them.
mix format✅.🤖 Generated with Claude Code
https://claude.ai/code/session_019NokzcR7BiAigPgC78zpk9