diff --git a/CONTEXT.md b/CONTEXT.md new file mode 100644 index 000000000..7e2b44c51 --- /dev/null +++ b/CONTEXT.md @@ -0,0 +1,35 @@ + + +# Apache Hamilton + +A Python library that builds a directed acyclic graph (DAG) of data transformations from plain Python functions, whose names are the nodes and whose parameters are the dependencies. + +This glossary records what words mean in this project where ordinary usage or generic tool knowledge would mislead. Add a term when the project settles one. + +## Language + +- **adapter** — three different things share this word. A *data adapter* is a `DataLoader`/`DataSaver` class registered by name in the adapter registry (`hamilton/registry.py`). A *legacy graph adapter* is a `HamiltonGraphAdapter` subclass (`hamilton/base.py`). A *lifecycle adapter* is a hook, method, or validator passed to `Builder.with_adapters` (`hamilton/lifecycle/`). Name which one you mean. +- **function vs node** — a *function* is what the user writes in a module; a *node* is one step in the DAG. One function yields zero, one, or many nodes (function modifiers expand, rename, or drop them), so never assume one-to-one. +- **function modifier** — Hamilton's name for its decorators (`@extract_columns`, `@parameterize`, `@config.when`, …). A function modifier changes how a function compiles into nodes when the graph is built; unlike a plain Python decorator it does not wrap the call. Modifiers resolve in a fixed order of phases, one per kind of modifier: resolvers, creators, injectors, expanders, transformers, and decorators (see [`writeups/adr/2611-03-function-modifiers-fixed-lifecycle.md`](writeups/adr/2611-03-function-modifiers-fixed-lifecycle.md)). `NodeDecorator` is the base class of the last of those phases, so "decorator" in the code can mean that phase rather than every modifier. +- **config / inputs / overrides** — *config* shapes the DAG and is fixed when the driver is built; *inputs* are per-run values for nodes nothing in the DAG produces; *overrides* replace values the DAG would otherwise compute, skipping their upstream work. Putting a value in the wrong one changes the graph or silently skips computation. +- **result builder** — combines the requested outputs into the return value of `execute()`. The default differs by entry point. The legacy `Driver(config, module)` uses `PandasDataFrameResult`. `driver.Builder` adds none, so `execute()` returns the outputs as a dict, as `DictResult` would. The same DAG can therefore return different types. +- **hook / method / validator** — the three kinds of lifecycle adapter. *Hooks* observe or augment execution and stack freely; *methods* replace one step of execution, and only one adapter may implement each method (a sync and an async implementation count separately); *validators* check the graph or its nodes when it is built. +- **materializer vs data loader/saver decorators** — *materializers* (`Driver.materialize` with `from_`/`to`) and `@load_from`/`@save_to` pick a registered data adapter by name and type. `@dataloader`/`@datasaver` instead wrap a user function that returns data plus metadata, with no registry involved. They are separate extension points. +- **extension vs plugin module** — `hamilton/plugins/_extensions.py` modules are *extensions*: the registry autoloads them to register types and data adapters. `hamilton/plugins/h_.py` modules are *plugins* the user imports explicitly for adapters and helpers. Both live in `hamilton/plugins/`, so a new integration has to choose which kind it is. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3b89072da..3fc6f2749 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -51,11 +51,9 @@ Additionally, the code should follow any stylistic and architectural guidelines prescribed by the project. For us here, this means you install a pre-commit hook and use the given style files. Basically, you should mimic the styles and patterns in the Apache Hamilton code-base. -In terms of getting setup to develop, we invite you to read our [developer setup guide](developer_setup.md). +In terms of getting set up to develop, we invite you to read our [developer setup guide](writeups/developer_setup.md). -## Using circleci CLI to run tests locally +## Finding your way around -1. Install the [circleci CLI](https://circleci.com/docs/2.0/local-cli/). -2. To run a circleci job locally, you should then, from the root of the repository, do - > circleci local execute --job=JOB_NAME -3. This assumes you have docker installed, it will pull the images, but otherwise will run the circleci job. +- [`CONTEXT.md`](CONTEXT.md) is a glossary of the words that mean something specific in Apache Hamilton, such as the three different things called an "adapter". Read it before naming something new. +- [`writeups/adr/`](writeups/adr/README.md) holds the project's decision records: why its foundations were built the way they are. Read the one that covers what you are changing before you change it. diff --git a/README.md b/README.md index f03afa50b..cc3b534f2 100644 --- a/README.md +++ b/README.md @@ -236,7 +236,7 @@ Apache Hamilton is released under the Apache 2.0 License. See [LICENSE](https:// ## 👨‍💻 Contributing We're very supportive of changes by new contributors, big or small! Make sure to discuss potential changes by creating an issue or commenting on an existing one before opening a pull request. Good first contributions include creating an example or an integration with your favorite Python library! - To contribute, checkout our [contributing guidelines](https://github.com/apache/hamilton/blob/main/CONTRIBUTING.md), our [developer setup guide](https://github.com/apache/hamilton/blob/main/developer_setup.md), and our [Code of Conduct](https://www.apache.org/foundation/policies/conduct.html). + To contribute, checkout our [contributing guidelines](https://github.com/apache/hamilton/blob/main/CONTRIBUTING.md), our [developer setup guide](https://github.com/apache/hamilton/blob/main/writeups/developer_setup.md), and our [Code of Conduct](https://www.apache.org/foundation/policies/conduct.html). ## 😎 Used by diff --git a/writeups/adr/2609-02-sql-metadata-keeps-1-0-0-filing.md b/writeups/adr/2609-02-sql-metadata-keeps-1-0-0-filing.md index 02fe5a14b..ca9576333 100644 --- a/writeups/adr/2609-02-sql-metadata-keeps-1-0-0-filing.md +++ b/writeups/adr/2609-02-sql-metadata-keeps-1-0-0-filing.md @@ -52,7 +52,7 @@ dataset. - Existing consumers of `sql_metadata` see the same `query`/`table_name` values for writes and for two-argument calls. Default-mode lineage names are unchanged for every string tested. - `query` does not always hold SQL. A written name containing `SELECT` sits there, so lineage code - must use `operation` and not the key alone (see ADR 0003). + must use `operation` and not the key alone (see [2609-03](2609-03-resolving-sql-writes-and-statements.md)). ## Alternatives considered diff --git a/writeups/adr/2609-03-resolving-sql-writes-and-statements.md b/writeups/adr/2609-03-resolving-sql-writes-and-statements.md index 43b05db42..910b713af 100644 --- a/writeups/adr/2609-03-resolving-sql-writes-and-statements.md +++ b/writeups/adr/2609-03-resolving-sql-writes-and-statements.md @@ -23,7 +23,7 @@ under the License. ## Context -Under ADR 0002, a write's string may sit under `query` or `table_name`. It may be a table name +Under [2609-02](2609-02-sql-metadata-keeps-1-0-0-filing.md), a write's string may sit under `query` or `table_name`. It may be a table name (`daily revenue`, `SELECT results`, `USER_SELECTIONS`) or, for custom savers, a statement (`INSERT INTO t SELECT ...`). `sql_datasets()` must name the tables written, and it must never invent a dataset named after SQL text or report a table name as the job's SQL. diff --git a/writeups/adr/2611-02-functions-define-nodes.md b/writeups/adr/2611-02-functions-define-nodes.md new file mode 100644 index 000000000..2c7c40018 --- /dev/null +++ b/writeups/adr/2611-02-functions-define-nodes.md @@ -0,0 +1,54 @@ + +# 2611-02. A function defines a node, and its parameter names are its dependencies + +**Status:** Reconstructed from codebase (decided 2019 at Stitch Fix: commit [f7c26ee8](https://github.com/apache/hamilton/commit/f7c26ee8c480abdeccda1cdc3f6fec4835a462ac) of 2019-07-25, the earliest in this history, already maps function names to nodes and parameters to dependencies; the other rules below arrived later, on the dates given with them) + +## Context + +Hamilton replaced a feature-engineering codebase in which a script built every column by mutating one shared dataframe. The origin post names the problems: "Column (i.e. feature) dependencies are hard to determine", "Documentation is hard", "Unit testing is hard", and "You need to execute and understand the entire script to get any columns (i.e. features)". The team "did not find any open-source tooling that would dramatically improve our capability to solve the aforementioned problems", and judged that "organizing code into functions is merely a tool, and does not provide adequate guard-rails". + +The 2020 README states the core choice: "Core to the design of hamilton is a 1:1 mapping of key (function name) to implementation. Rather than defining flexible, reusable transformations, Hamilton aims for DAG clarity, easy modifications, and documentation." + +This record was reconstructed from the code of commit [f7c26ee8](https://github.com/apache/hamilton/commit/f7c26ee8c480abdeccda1cdc3f6fec4835a462ac), the project's first README (commit [3b70736b](https://github.com/apache/hamilton/commit/3b70736ba3a40348581de9d113291d00a6814abe), 2020-08-21), the Stitch Fix origin post ("[Functions + DAGs: introducing Hamilton](https://multithreaded.stitchfix.com/blog/2021/10/14/functions-dags-hamilton/)", 2021-10-14, linked at [`README.md:245`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/README.md#L245)), [`docs/concepts/node.rst`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/docs/concepts/node.rst), and [`writeups/basics.md`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/writeups/basics.md). The design is older than this repository's history, and no public discussion from before 2019-07-25 exists. [f7c26ee8](https://github.com/apache/hamilton/commit/f7c26ee8c480abdeccda1cdc3f6fec4835a462ac) , whose message lists "Refactored hamilton prototype to function on customizable modules", already builds one node per function, takes dependencies from parameter names, skips functions whose name starts with `_`, requires names and types to match, and refuses a function defined twice. The 2020 README is the earliest written reasoning; `node.rst` was written in 2024 ([#659](https://github.com/apache/hamilton/pull/659)), so its reasoning is the rationale as later documented. + +## Decision + +- Hamilton derives the directed acyclic graph (DAG) by inspecting function definitions. A user never declares the graph separately. The function name is the node name and the return annotation is the node type. The parameter names and annotations are the node's dependencies, and the docstring is its description ([`docs/concepts/node.rst:20`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/docs/concepts/node.rst#L20) and the table under "Anatomy of a node", [`node.rst:76`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/docs/concepts/node.rst#L76)). The docstring became the description in 2019-08 ([8c8f7843](https://github.com/apache/hamilton/commit/8c8f7843c87eb68ae125bc461c302a42a269350d): "We want to also easily display documentation relating to business logic that gets encoded"). +- A node name is a globally unique key. A DAG can have only one function named `sum_a_b` ([`writeups/basics.md:69-71`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/writeups/basics.md#L69-L71)). A name defined in two modules is an error unless the caller opts in with `Builder.allow_module_overrides()` ([#1134](https://github.com/apache/hamilton/pull/1134), 2024-09), in which case the later module wins ([`docs/concepts/builder.rst:78`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/docs/concepts/builder.rst#L78)). +- A function whose name starts with `_` is a helper and does not become a node ([`docs/concepts/node.rst:45`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/docs/concepts/node.rst#L45)). The 2020 README gives the reason: "Hamilton tries to make use of every function in a module, so this allows us to easily indicate helper functions". Since 2024 the `@hamilton_exclude` decorator does the same for the exceptional case ([`configuration.py`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/function_modifiers/configuration.py#L278-L283)). +- A parameter that no function produces is an input. The caller must supply it, unless the parameter has a default. A default makes it optional ([`DependencyType.OPTIONAL`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/node.py#L52-L60)), and when the caller does not supply a value, execution leaves the argument out so the function's default applies ([`hamilton/execution/graph_functions.py:211-223`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/execution/graph_functions.py#L211-L223)). Optional inputs arrived in 2020-01 ([662dceef](https://github.com/apache/hamilton/commit/662dceefe2d8f4169c1faa1845612eaf471d6d20)) and were completed in [stitchfix/hamilton#96](https://github.com/stitchfix/hamilton/pull/96) (2022-03). +- Type annotations are required. Hamilton uses them to check that each producer matches its consumers ([`writeups/basics.md:75`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/writeups/basics.md#L75)). Requiring them was a usability fix of 2019-08 ([11dec583](https://github.com/apache/hamilton/commit/11dec58393acf19c808755194b3143354fe988ef): "This is a usability improvement. I forgot to add a type and was scratching my head"). + +## Consequences + +- A function's code shows its place in the dataflow. `node.rst` argues this under "Readability" ([line 119](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/docs/concepts/node.rst#L119)), against frameworks that define steps first and wire them elsewhere. Under "Maintainability" ([line 123](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/docs/concepts/node.rst#L123)) it argues that a change to a node definition reaches every dataflow that uses it. +- Renaming a function or a parameter changes the graph. That rename is an application programming interface (API) change for any caller that requests the node or supplies it as an input. +- Function names should be nouns that describe the result, not verbs, because the name is how the node is queried ([`docs/concepts/node.rst:67`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/docs/concepts/node.rst#L67)). +- Reusing one function body under several node names requires a function modifier (see 2611-03). [`writeups/basics.md:70`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/writeups/basics.md#L70) admits that the unique-name rule "is not optimal for functionality reuse". It accepts that cost because the rule "makes it extremely easy to learn exactly how a node in the DAG is generated". +- A second function with the same name inside one module silently replaces the first, because Hamilton reads a module's members ([`hamilton/graph_utils.py:43`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/graph_utils.py#L43)), where only the last definition survives. The refusal of duplicates applies across modules and across modifier outputs. This was known from the start ([36b18e54](https://github.com/apache/hamilton/commit/36b18e54f1163a653843df4d9d6c34e5e30c3394), 2019-09: "No two functions can have the same name (or they'll be overwritten)"), and a 2022 issue asking to catch it was closed because "we don't actually get returned multiple functions to know there are duplicates" ([stitchfix/hamilton#193](https://github.com/stitchfix/hamilton/issues/193)). + +## Alternatives considered + +- **Flexible, reusable transformations** rather than one implementation per name. Rejected in the 2020 README, for "DAG clarity, easy modifications, and documentation". +- **Define steps, then wire them explicitly**, as in "popular orchestration / pipeline / workflow frameworks (Airflow, Kedro, Prefect, VertexAI, SageMaker, etc.)" ([`docs/concepts/node.rst:113`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/docs/concepts/node.rst#L113)). Rejected for readability and maintainability, in the 2024 documentation's words. +- **Adopt existing open-source tooling.** The origin post says the team surveyed "a variety of offerings" and found none that solved the problems. Which tools it evaluated is not recorded. +- **Plain functions with no framework.** The origin post rejected them as "merely a tool" without guard-rails. +- **Dependencies selected by tag rather than by name.** Proposed in 2022 ([stitchfix/hamilton#226](https://github.com/stitchfix/hamilton/issues/226)). Elijah ben Izzy noted that it broke two of Hamilton's rules: "(a) that we can always determine the exact dependencies and (b) that functions have a fixed number of dependencies" ([comment](https://github.com/stitchfix/hamilton/issues/226#issuecomment-1301002225)), and Stefan Krawczyk's "bias here is for an API that's readable and understandable by people with little context" ([comment](https://github.com/stitchfix/hamilton/issues/226#issuecomment-1315930049)). It was not built. +- **Redefining a node under the same name.** Proposed in 2023 ([stitchfix/hamilton#290](https://github.com/stitchfix/hamilton/issues/290)): it "makes implementation/onboarding easier, but doesn't help readability/maintainability of pipeline". Closed in favor of `@pipe`, which "gaurentees immutability, but allows chaining/redefinition" ([comment](https://github.com/apache/hamilton/issues/65#issuecomment-1769990243), on the migrated issue). diff --git a/writeups/adr/2611-03-function-modifiers-fixed-lifecycle.md b/writeups/adr/2611-03-function-modifiers-fixed-lifecycle.md new file mode 100644 index 000000000..9f4f931e0 --- /dev/null +++ b/writeups/adr/2611-03-function-modifiers-fixed-lifecycle.md @@ -0,0 +1,62 @@ + +# 2611-03. Function modifiers annotate a function and resolve in a fixed lifecycle at graph-build time + +**Status:** Reconstructed from codebase (decided 2021-12 in commit [bc7c089d](https://github.com/apache/hamilton/commit/bc7c089d3715ebd5936dd0beed8428ecf7e69ea8), "Implements opinionated decorator lifecycle", merged 2022-02 in [stitchfix/hamilton#28](https://github.com/stitchfix/hamilton/pull/28) for release 1.3.0; phases added since are dated below) + +## Context + +**Rationale partly recovered.** Why stitchfix/hamilton#28 put the phases in this order, and which other orders were weighed then, are not recorded; the pull requests that added phases later say why they did. + +By 2611-02, one function is one node. Decorators were needed where one function should become zero nodes (`@config.when`), several nodes (`@parameterize`, `@extract_columns`), or nodes with extra metadata or checks (`@tag`, `@check_output`). The commit "adds to the set of decorators that can be applied to a node, and puts them in a uniform setting". + +Both people on stitchfix/hamilton#28 described it as mostly a refactor. The reviewer, Stefan Krawczyk, asked "Otherwise the decorator lifecycle is largely a refactor of existing behavior, right?" ([review](https://github.com/stitchfix/hamilton/pull/28#pullrequestreview-829399377)), and the author, Elijah ben Izzy, answered that it was "a refactor of existing behavior. Technically this allows models + parametrization together (whereas before it wouldn't)" ([comment](https://github.com/stitchfix/hamilton/pull/28#issuecomment-991376043)). The pieces it unified date from 2019: +- Expanders, and attaching to a function instead of wrapping it: [889bc1c8](https://github.com/apache/hamilton/commit/889bc1c8a6a7ab44540949cf830d9efb315c7113) "Modified the graph module to ask for the nodes a function produces (when annotated), rather than assuming each function maps to one node." +- Resolvers running first: [36b18e54](https://github.com/apache/hamilton/commit/36b18e54f1163a653843df4d9d6c34e5e30c3394) "we create a new abstraction, FunctionResolver, that allows us to resolve a function before modifying it", while noting that a proper design would need "a better set of abstractions". + +Sensitivity to stacking order was one concrete motive. A commit in stitchfix/hamilton#28 ([2fb3c60a](https://github.com/apache/hamilton/commit/2fb3c60a951c337974c552beb5044f345eb34bff)) records that `@config.when` renamed the function, but "this broke depending on decorator ordering". + +This record was reconstructed from commit [bc7c089d](https://github.com/apache/hamilton/commit/bc7c089d3715ebd5936dd0beed8428ecf7e69ea8) (2021-12-08) and the pull request that carried it, [stitchfix/hamilton#28](https://github.com/stitchfix/hamilton/pull/28) (from the project's home before it moved to the Apache Software Foundation, merged 2022-02-06); from the 2019 commits that introduced resolvers and expanders; from the [`resolve_nodes` docstring](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/function_modifiers/base.py#L830), [`writeups/decorators.md:640-665`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/writeups/decorators.md#L640-L665) ("A mental model for decorators"), and [`docs/concepts/function-modifiers.rst:25`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/docs/concepts/function-modifiers.rst#L25). The sources name two goals: one "uniform setting" for all decorators, and decorators that "should be insensitive to ordering". + +## Decision + +- A function modifier does not wrap the function. `NodeTransformLifecycle.__call__` validates the function and appends the modifier object to a list attribute on it, named for the modifier's lifecycle phase ([`hamilton/function_modifiers/base.py:112-132`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/function_modifiers/base.py#L112-L132)). The function stays callable as written. +- When the graph is built, [`resolve_nodes`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/function_modifiers/base.py#L830) (called from [`hamilton/graph.py:190`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/graph.py#L190)) applies the attached function modifiers by phase, in a fixed order between phases: + 1. resolvers (`NodeResolver`, such as `@config.when`) decide whether the function exists in this graph, and any resolver returning `None` drops it; dynamic resolvers (`@resolve`, added 2023-03 in [apache/hamilton#108](https://github.com/apache/hamilton/pull/108)) first turn config into further function modifiers, which join their phases after the statically attached ones ([`get_node_decorators`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/function_modifiers/base.py#L754-L766)); + 2. exactly one creator (`NodeCreator`, by default the plain function) produces the initial nodes; + 3. injectors (`NodeInjector`, added 2023-07 in [#221](https://github.com/apache/hamilton/pull/221)) feed values into the subdag; injection had been a creator, which "could only be run once", and as a subdag modifier it can run several times ([6c6528e3](https://github.com/apache/hamilton/commit/6c6528e3218d2c87c52bafa69de512573123ba64)); + 4. expanders (`NodeExpander`): any pre-expanders (added 2026-09 in [#1650](https://github.com/apache/hamilton/pull/1650), so that "`@inject` can run before regular expanders such as `@parameterize`"), then at most one regular expander, turn one node into many; + 5. transformers (`NodeTransformer`), then decorators (`NodeDecorator`, added 2022-02 for tags in [stitchfix/hamilton#83](https://github.com/stitchfix/hamilton/pull/83)), modify the resulting nodes. + + Within one phase, function modifiers apply in the order they were stacked, from the bottom up ([`base.py:129`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/function_modifiers/base.py#L129)). +- A layering function modifier chooses which of the generated nodes it modifies with `target_`, added in [stitchfix/hamilton#316](https://github.com/stitchfix/hamilton/pull/316) ([b848a72d](https://github.com/stitchfix/hamilton/commit/b848a72d37c3b365c5d5403debcb5551c2a3f82a)). `None` means the "final" nodes, that is, nodes no other node from the same function depends on. `...` means all nodes, and a string or list names nodes explicitly ([`writeups/decorators.md:657-665`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/writeups/decorators.md#L657-L665), [`NON_FINAL_TAG`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/function_modifiers/base.py#L386)). In review, Stefan Krawczyk found that "going between function and nodes is confusing", and Elijah ben Izzy answered "let's revisit that and see how users react" ([review comment](https://github.com/stitchfix/hamilton/pull/316#discussion_r1110472921)). The default follows a 2022 proposal that "tags associate to only the last node in a subdag created by a node decorator" ([stitchfix/hamilton#4](https://github.com/stitchfix/hamilton/issues/4#issuecomment-1048476479)). +- Each function modifier sees only the config keys it declares ([`filter_config`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/function_modifiers/base.py#L725)). The commit that added it, in the same pull request ([9bdcaf1f](https://github.com/apache/hamilton/commit/9bdcaf1fbfa9183c3729d2cf368d582fc09f671e), 2023-02) gives the reason: "This allows for inspection/visibility, and will be essential in checkpointing/storage options. We want decorators to declare these upfront, rather than relying on the internals of the decorators to break, enabling clearer error messages". + +## Consequences + +- One function maps to zero, one, or many nodes ([`writeups/decorators.md:648-650`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/writeups/decorators.md#L648-L650)). "Function" and "node" are different terms, and code that walks the graph must not assume they correspond one to one. +- Hamilton decorators "should be insensitive to ordering" ([`docs/concepts/function-modifiers.rst:25`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/docs/concepts/function-modifiers.rst#L25)), and the same page warns that non-Hamilton decorators, such as `@retries` or `@time`, are not. Insensitivity is an aim rather than a guarantee: on one combination that failed, Elijah ben Izzy wrote there was "no inherent reason why this should work -- its an implementation detail that it doesn't" ([comment](https://github.com/apache/hamilton/issues/160#issuecomment-1535150079)), and #1650 made that combination work in 2026; [#748](https://github.com/apache/hamilton/issues/748) (`@inject` with `@pipe`) is open; and [#57](https://github.com/apache/hamilton/issues/57), "Clarify behavior of decorator ordering", is open, asking the project to "make clear our philosophy and resolution method". +- Config acts during construction of the directed acyclic graph (DAG), through resolvers, so it is fixed when the Driver is built. [`writeups/basics.md:137-138`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/writeups/basics.md#L137-L138) states the reason: config "is used not just to feed data to the DAG, but also to determine the structure of the DAG. As such, it is passed in to the constructor". Changing config means building a new Driver ([`docs/concepts/builder.rst:96`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/docs/concepts/builder.rst#L96)). Config must not be used to replace computed values; `overrides` on `execute()` does that ([`writeups/basics.md:140-142`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/writeups/basics.md#L140-L142)). +- A new function modifier must fit one of the phases. Two regular expanders on one function are rejected (the `len(regular_expanders) > 1` check in [`resolve_nodes`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/function_modifiers/base.py#L879)). + +## Alternatives considered + +- **Other phase orders, or applying function modifiers in stacking order.** For the order stitchfix/hamilton#28 set, not recorded: no issue, pull request or document found weighs them. +- **An `augment` decorator.** Added in stitchfix/hamilton#28 and removed before merge ([e7866102](https://github.com/apache/hamilton/commit/e78661022a7deef84e6c02c8029986907674141a)). The reviewer objected that "it is not clear at all what the outputs are here" ([review comment](https://github.com/stitchfix/hamilton/pull/28#discussion_r800111209)); the author agreed it was "not necessary. It loses type information." ([review comment](https://github.com/stitchfix/hamilton/pull/28#discussion_r800115984)). +- **A linter that reorders decorators.** Floated on #57 ("can we create a linter that reorders decorators?") and not built. diff --git a/writeups/adr/2611-04-version-gated-deprecations.md b/writeups/adr/2611-04-version-gated-deprecations.md new file mode 100644 index 000000000..54757c0f1 --- /dev/null +++ b/writeups/adr/2611-04-version-gated-deprecations.md @@ -0,0 +1,52 @@ + +# 2611-04. Deprecations name the major release that removes them, through a warn/fail version contract + +**Status:** Reconstructed from codebase (decided 2022-07 in commit [d1edda18](https://github.com/apache/hamilton/commit/d1edda187e53dca77f537192fa3cf5f3958e3944), "Adds @deprecated decorator", merged 2022-08 in [stitchfix/hamilton#163](https://github.com/stitchfix/hamilton/pull/163)) + +## Context + +The decorator was built to retire the `@parametrized*` decorators, which stitchfix/hamilton#163 replaced with `@parameterize`, `@parameterize_values` and `@parameterize_sources`: the author's checklist on that pull request includes "Add a `@deprecated` decorator for decorators" ([comment](https://github.com/stitchfix/hamilton/pull/163#issuecomment-1200063743)), and its first uses warned from 1.10.0 and failed from 2.0.0. Removal in 2.0 was already the practice by then: commit [b4e874bd](https://github.com/apache/hamilton/commit/b4e874bd9ffee327d4faccd8092712ff9c5ba537) (2022-04) put a plain log warning on `parameterized_input` saying "It will be removed in a 2.0.0 release." The decorator turned that convention into a contract the code checks. + +The commit author "decided to build my own as I wanted to fully manage the lifecycle of items", and wrote that the mechanism "forces us into contracts. Though we can break them, I like the idea of making deprecations a planned event." + +This record was reconstructed from commit [d1edda18](https://github.com/apache/hamilton/commit/d1edda187e53dca77f537192fa3cf5f3958e3944) (2022-07-30), the pull request that carried it, [stitchfix/hamilton#163](https://github.com/stitchfix/hamilton/pull/163) ("Full parametrized decorator", from the project's home before it moved to the Apache Software Foundation, merged 2022-08-15), and [`hamilton/dev_utils/deprecation.py`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/dev_utils/deprecation.py). The commit message gives the reasoning. The decorator has since changed in [apache/hamilton#1802](https://github.com/apache/hamilton/pull/1802), [#1803](https://github.com/apache/hamilton/pull/1803) and [#1804](https://github.com/apache/hamilton/pull/1804); the Decision below describes it as it is now, and the Consequences say what those changes did. + +## Decision + +- An application programming interface (API) deprecation that carries a version contract goes through `hamilton.dev_utils.deprecation.deprecated`, a decorator for functions and callable classes. It takes `warn_starting`, `fail_starting`, `use_this`, `explanation`, and `migration_guide`. +- Its behavior depends on the installed Hamilton version (`hamilton.version.VERSION`). From `warn_starting` on, a call warns; from `fail_starting` on, it raises `DeprecationError`. Both bounds are inclusive ([`_should_warn` and `_should_fail`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/dev_utils/deprecation.py#L178-L182) use `>=`), and a pre-release counts as its release, so 2.0.0rc1 fails too. The commit intended that from the start ("no funky versions or rc versions (the rc gets stripped)"), and in review the author agreed to test "the parser that strips it" ([review comment](https://github.com/stitchfix/hamilton/pull/163#discussion_r939754991)). +- The default `warn_action` issues a `FutureWarning` attributed to the code that called the deprecated API, so the standard `warnings` filters apply (`python -W error::FutureWarning` turns it into an error). +- A `fail_starting` of 1.0.0 or later must be a major release (`x.0.0`); one below 1.0.0 is exempt. In every case it must not come before `warn_starting` ([`_validate_fail_starting`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/dev_utils/deprecation.py#L143-L152)). +- A deprecation without a replacement (`use_this=None`) must give a `migration_guide` ([`__post_init__`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/dev_utils/deprecation.py#L132-L141)). A reviewer on stitchfix/hamilton#163 asked for "a clearer migration section that we can link to so people know what to do and until when" ([review](https://github.com/stitchfix/hamilton/pull/163#pullrequestreview-1064323389)), and the answer was a migration section in the decorator documentation ([83c3c6e0](https://github.com/apache/hamilton/commit/83c3c6e0d90ee1048366fe3e4dcbe112234976f9)). +- The docstring states the contract: "if you promise to deprecate something by X.0, then do it!" ([line 101](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/dev_utils/deprecation.py#L101)). + +## Consequences + +- A deprecation is a promise of the major version that removes the item. Current uses are in [`hamilton/driver.py`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/driver.py#L606), [`hamilton/function_modifiers/expanders.py`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/function_modifiers/expanders.py#L470), [`hamilton/function_modifiers/macros.py`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/function_modifiers/macros.py#L255), and [`hamilton/telemetry.py`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/telemetry.py#L29). All of them name `fail_starting=(2, 0, 0)`. +- Not every deprecation uses the decorator. Some call `warnings.warn` directly and carry no fail version, so nothing makes them raise in 2.0.0. They include module shims that issue a `FutureWarning` on import: [`hamilton/function_modifiers_base.py:25`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/function_modifiers_base.py#L25), [`hamilton/plugins/polars_implementations.py:22`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/plugins/polars_implementations.py#L22), and five modules in `hamilton/experimental/` ([`h_async.py:22`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/experimental/h_async.py#L22), [`h_cache.py:35`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/experimental/h_cache.py#L35), [`h_dask.py:22`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/experimental/h_dask.py#L22), [`h_ray.py:22`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/experimental/h_ray.py#L22), and [`h_spark.py:26`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/experimental/h_spark.py#L26)). The `CacheAdapter` ([`hamilton/lifecycle/default.py:371`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/lifecycle/default.py#L371)) and `DiskCacheAdapter` ([`hamilton/plugins/h_diskcache.py:108`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/plugins/h_diskcache.py#L108)) constructors also issue a `FutureWarning`. `display_graph=True` (`hamilton/driver.py` lines [549](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/driver.py#L549), [641](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/driver.py#L641) and [717](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/driver.py#L717)) and `FunctionGraph.get_impacted_nodes` ([`hamilton/graph.py:1001`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/graph.py#L1001)) issue a `DeprecationWarning` instead. Most of these messages name 2.0 as the removal release, but only in their text. The `polars_implementations`, `h_dask`, `h_ray`, and `h_spark` shims name no release. The shims began as log lines: the `function_modifiers_base` one comes from a reviewer's suggested `logger.warning` on [stitchfix/hamilton#237](https://github.com/stitchfix/hamilton/pull/237#discussion_r1032997523), and #1804 turned the seven shims and both cache adapters into `FutureWarning`s. Why they bypass the decorator is not recorded. +- Until #1802 both comparisons were strict (`>`), so an item with `fail_starting=(2, 0, 0)` would only have warned on 2.0.0 and first raised on 2.0.1, against the docstring's "by X.0". #1802 made them inclusive, so such an item raises on 2.0.0. +- Until #1804 the default `warn_action` was `logger.warning`, so `-W error` and pytest warning filters did not catch these warnings. #1804 made it a `FutureWarning`, and #1803 builds every message in one place: a warning reads "… is deprecated and will stop working in hamilton version: …". +- A deprecation that passes its own `current_version` freezes its clock. `pipe` and `dynamic_transform` did, and #1804 found that "`pipe` warned on every release, `dynamic_transform` never warned, and neither would ever fail". It removed the pins and added [`tests/test_deprecation_schedules.py`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/tests/test_deprecation_schedules.py), which fails when a production `deprecated(...)` call passes `current_version`. Both #1803 and #1804 leave one design question open: "Reading the version when the API is called would be a cleaner design, but it changes the decorator's interface and needs its own decision." +- Replacing this mechanism takes a record that supersedes this one (`writeups/adr/README.md`). + +## Alternatives considered + +- **An existing deprecation library or bare `warnings.warn`.** The commit says only that the author "decided to build my own", to manage the whole lifecycle. The body, comments and reviews of stitchfix/hamilton#163 name no deprecation library. The one third-party library the pull request mentions is wrapt, weighed for wrapping both functions and callable classes rather than as a deprecation library: commit [5e9a71f4](https://github.com/apache/hamilton/commit/5e9a71f40eb77f094c74b3bdd2306b749f8640ed), in the same pull request, says "a third party library like wrapt might help but I wanted to keep it simple." +- **A different warning category.** #1804 weighed `DeprecationWarning`, which "Python hides … by default unless the warning is attributed to `__main__`", and a Hamilton-specific subclass, rejected because "it adds a public name", and chose `FutureWarning`. A subclass named `RemovedInHamilton2Warning` had been proposed in a contributor's work plan on [apache/hamilton#918](https://github.com/apache/hamilton/issues/918#issuecomment-4692760418). diff --git a/writeups/adr/2611-05-extension-registry-autoload.md b/writeups/adr/2611-05-extension-registry-autoload.md new file mode 100644 index 000000000..cf491d492 --- /dev/null +++ b/writeups/adr/2611-05-extension-registry-autoload.md @@ -0,0 +1,54 @@ + +# 2611-05. Library integrations load through a dependency-free registry, autoloaded unless the user opts out + +**Status:** Reconstructed from codebase (registry decided 2023-01 in commit [c8f8cb56](https://github.com/apache/hamilton/commit/c8f8cb56543e9ff504b62b747caaa22a47ec9d4b), in [stitchfix/hamilton#273](https://github.com/stitchfix/hamilton/pull/273); opt-out autoload decided 2024-08 in [apache/hamilton#1095](https://github.com/apache/hamilton/pull/1095)) + +## Context + +Hamilton supports many dataframe and model libraries (pandas, polars, Spark, Dask, xgboost, mlflow, and others). Features such as `@extract_columns` and the data loaders and savers (2611-06) need to know each library's types. That knowledge lives in `hamilton/plugins/_extensions.py` modules, and only works if the library is installed. + +stitchfix/hamilton#273 states the problem it solved: "We had a problem in the decorators (notably extract_columns) that assumed pandas. We need to abstract this out so that other dataframe libraries could be easily used with the decorator." On the polars example that preceded it ([stitchfix/hamilton#263](https://github.com/stitchfix/hamilton/pull/263)), one maintainer proposed that plugins register their own decorator implementations; the other preferred abstracting the "dataframe" instead of the decorator ([review comment](https://github.com/stitchfix/hamilton/pull/263#discussion_r1059830726)), which became the per-library `get_column` and `fill_with_scalar` functions, and wanted the integrations kept apart because they "don't want to add polars as a dependency to main package" ([review comment](https://github.com/stitchfix/hamilton/pull/263#discussion_r1059804731)). + +The pull request first put registration in `hamilton.base` ([c9071135](https://github.com/apache/hamilton/commit/c907113560fccbb043c649da66501093e70bf331), 2023-01-12). A reviewer asked "Isn't this a circular dependency?" ([review comment](https://github.com/stitchfix/hamilton/pull/273#discussion_r1070018235)), and hours later [c8f8cb56](https://github.com/apache/hamilton/commit/c8f8cb56543e9ff504b62b747caaa22a47ec9d4b) moved registration into a new `registry` module, which "is not allowed to depend on anything else": extensions import `registry`, and because `registry` imports nothing from Hamilton, loading them creates no circular import. + +#1059 (2024-07) reported that importing the driver "takes approximately 4-5 seconds". A maintainer answered that libraries a user has not imported, "Hamilton shouldn't either" ([comment](https://github.com/apache/hamilton/issues/1059#issuecomment-2249671443)). #1095 records the cost of the greedy design: `import hamilton.driver` "tries to import all the extension modules available to the users ... even if these libraries aren't used in the dataflow", which "can introduce slowdowns, memory usage (many Mbs), and warning messages by 3rd party libraries". The how-to it added puts the saving from opting out at "from 2-3 sec to less than 0.5 sec" ([`docs/how-tos/extensions-autoloading.rst`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/docs/how-tos/extensions-autoloading.rst#L19)). + +This record was reconstructed from commit [c8f8cb56](https://github.com/apache/hamilton/commit/c8f8cb56543e9ff504b62b747caaa22a47ec9d4b) (2023-01-13, "Refactors and creates a registry module") and the pull request that carried it, [stitchfix/hamilton#273](https://github.com/stitchfix/hamilton/pull/273) ("Introduces *_extensions…", from the project's home before it moved to the Apache Software Foundation, merged 2023-01-16); from [apache/hamilton#1095](https://github.com/apache/hamilton/pull/1095) (merged as [8251ae48](https://github.com/apache/hamilton/commit/8251ae48f90b9a715daa96c419109458494d254e)) and the issue behind it, [#1059](https://github.com/apache/hamilton/issues/1059); and from [`hamilton/registry.py`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/registry.py) and [`pyproject.toml`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/pyproject.toml#L48-L56). + +## Decision + +- [`hamilton/registry.py`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/registry.py#L18-L25) imports only the standard library. It lists the known extensions ([`ExtensionName`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/registry.py#L31-L53)) and holds the per-library type tables, plus [`LOADER_REGISTRY` and `SAVER_REGISTRY`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/registry.py#L220-L221). +- [`registry.initialize()`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/registry.py#L100-L114) tries to import every listed extension and skips any that raises `ImportError` (its library is missing), `NotImplementedError`, or a `Warning` (a warning raised as an error, caught since [157ecd60](https://github.com/apache/hamilton/commit/157ecd60c8371be6c479bc2dbed470487c948c74)). `hamilton.function_modifiers.base` calls it at import time ([`hamilton/function_modifiers/base.py:41`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/function_modifiers/base.py#L41)); commit [2489423d](https://github.com/apache/hamilton/commit/2489423dd6ccb74a38d2266536424356c7d692b3), in stitchfix/hamilton#273, put the call there because "Ray serialization is the cause of this move" and "decorators are the only thing that need the registry right now". +- Autoload stays the default. Users can opt out with [`registry.disable_autoload()`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/registry.py#L117-L122), the `HAMILTON_AUTOLOAD_EXTENSIONS=0` environment variable, or a persistent [`~/.hamilton.conf`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/registry.py#L56) entry set by the `hamilton-disable-autoload-extensions` command ([`pyproject.toml`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/pyproject.toml#L209-L210)). A user who opts out loads what they need with [`registry.load_extension(name)`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/registry.py#L79-L97) or by importing the extension module. +- Third-party libraries an integration needs are optional extras in `pyproject.toml` (`[project.optional-dependencies]`), not core dependencies. + +## Consequences + +- A new integration adds an `ExtensionName` entry and a `_extensions.py` module. That module must provide `register_types`, `get_column_`, and `fill_with_scalar_`, unless it sets `COLUMN_FRIENDLY_DF_TYPE = False` (`registry.load_extension`). `register_types` answers a review question on stitchfix/hamilton#273, "Might be nice to have modules add their own types?" ([review comment](https://github.com/stitchfix/hamilton/pull/273#discussion_r1070016188)). +- Importing Hamilton in an environment that has many libraries installed still pays for the import of every matching extension, unless the user opts out. Greedy loading has broken imports outright: users with pydantic older than 2.0 installed hit errors "even if not using Pydantic" until pydantic moved into its own extension ([apache/hamilton#1152](https://github.com/apache/hamilton/pull/1152)). +- Disabling autoload must happen before `hamilton.driver` is imported (`disable_autoload` docstring). +- Opting out does not remove the import cost of pandas and numpy, which are still core dependencies ([`pyproject.toml`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/pyproject.toml#L48-L54), `dependencies`); [apache/hamilton#1246](https://github.com/apache/hamilton/issues/1246), on profiling the driver's imports, is open. A maintainer wrote on [apache/hamilton#918](https://github.com/apache/hamilton/issues/918#issuecomment-2150392609) (2024-06-05) that pandas is "only left there for legacy reasons" and that "the library should have zero dependencies". That issue is open, and removing pandas and numpy is not decided. + +## Alternatives considered + +- **Make loading opt-in by default.** Rejected in #1095: "Since many features (`@extract_columns`, materializers, and more) rely on this extension registry, changing the default behavior would introduce many hard-to-track breaking changes." +- **Load only the extensions whose library the user has already imported.** #1095's first version did this. A reviewer asked why that clause was needed ([review comment](https://github.com/apache/hamilton/pull/1095#discussion_r1717623413)), and the author proposed three modes, greedy, lazy and disabled, noting "Lazy might be more valuable for development and Disabled better in prod" ([review comment](https://github.com/apache/hamilton/pull/1095#discussion_r1718547747)). The lazy clause was removed before merge ([5bf96dc1](https://github.com/apache/hamilton/commit/5bf96dc1d90909b2f058f437afb92a416c8a9a04)); why is not recorded. +- **Keep registration in `hamilton.base`.** The first draft of stitchfix/hamilton#273 did; [c8f8cb56](https://github.com/apache/hamilton/commit/c8f8cb56543e9ff504b62b747caaa22a47ec9d4b) moved it out to avoid circular imports between core and the extension modules. +- **An object as the extension interface.** The body of stitchfix/hamilton#273 weighed an object one has to implement, and judged "this abstraction right now is simpler -- the housing unit is the module", with "trying to import the modules and having them determine how to error" as the loading rule. diff --git a/writeups/adr/2611-06-data-adapters-and-materialization.md b/writeups/adr/2611-06-data-adapters-and-materialization.md new file mode 100644 index 000000000..2fd517a5d --- /dev/null +++ b/writeups/adr/2611-06-data-adapters-and-materialization.md @@ -0,0 +1,52 @@ + +# 2611-06. Data I/O is done by registered loader and saver classes that become nodes in the DAG + +**Status:** Reconstructed from codebase (data adapters decided 2023-04 in [apache/hamilton#128](https://github.com/apache/hamilton/pull/128) / commit [f4eafcaf](https://github.com/apache/hamilton/commit/f4eafcaf36ddbb4fffe643553aff055f53c39a5d); driver-level materialization specified 2023-07 in [#235](https://github.com/apache/hamilton/issues/235) and merged 2023-08 with [#220](https://github.com/apache/hamilton/pull/220) / commit [134b7ecc](https://github.com/apache/hamilton/commit/134b7ecc2623bc40ff512e14fe78d380fc644250)) + +## Context + +Before this, users loaded and saved data inside node functions or in driver code. [`docs/concepts/materialization.rst:52-56`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/docs/concepts/materialization.rst#L52-L56), written in 2024 with `Builder.with_materializers()` ([2924ed0e](https://github.com/apache/hamilton/commit/2924ed0e3323489554f3017c2ebb046a1592177e), #911), names the three limitations the design "aims to solve". **Redundancy**: deduplicate loading and saving code. **Observability**: include loading and saving in the dataflow, the directed acyclic graph (DAG) Hamilton builds, so that it can be seen and hooked. **Flexibility**: change loading and saving behavior without editing the dataflow. The design document anticipated the last one: "connections should be injected. they're execution time specific, i.e. instantiated at driver time." The design's author set out the reasoning at length in a 2023-11 blog post linked from `examples/decoupling_io/README.md`, "[Separate data I/O from transformation](https://blog.dagworks.io/p/separate-data-io-from-transformation)", which calls materializers "the primary tool we will leverage to separate I/O from transformations". + +The spec on #235 lists the requirements for driver-level materialization. It "actually modifies the DAG so we get nodes that are traceable/manageable", "reuses the data adapter framework", "is simple/easy to call", "handles more than just pandas", "handles API keys for saving in a graceful way", and "Handles custom joining of results prior to saving". + +This record was reconstructed from commit [f4eafcaf](https://github.com/apache/hamilton/commit/f4eafcaf36ddbb4fffe643553aff055f53c39a5d) (2023-04-01, "Adds implementation of data loaders"), the [spec comment on issue #235](https://github.com/apache/hamilton/issues/235#issuecomment-1650793866) (2023-07-26), commit [134b7ecc](https://github.com/apache/hamilton/commit/134b7ecc2623bc40ff512e14fe78d380fc644250), the issues and pull requests named below, [`docs/concepts/materialization.rst`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/docs/concepts/materialization.rst), [`hamilton/io/data_adapters.py`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/io/data_adapters.py), and [`hamilton/registry.py`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/registry.py#L220-L233). Materialization was built in [#236](https://github.com/apache/hamilton/pull/236), which merged into the `orchestration` branch, and reached `main` with #220. The bodies of #128 and #236 are unfilled templates, their reviews cover only documentation and naming, and #220's body says only "See commit history". The design document linked from [apache/hamilton#46](https://github.com/apache/hamilton/issues/46) (migrated from [stitchfix/hamilton#197](https://github.com/stitchfix/hamilton/issues/197)) is titled "SQL/DBT Integration"; it is about database connections rather than data adapters in general, and its comments carry no authors or dates. + +## Decision + +- A data loader or saver is a dataclass that subclasses `DataLoader` or `DataSaver` (`hamilton/io/data_adapters.py` lines [132](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/io/data_adapters.py#L132) and [173](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/io/data_adapters.py#L173)). It is constructed from keyword arguments and declares a `name()` and the `applicable_types()` it handles. Hamilton checks that subclasses are dataclasses ([`AdapterCommon._ensure_dataclass`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/io/data_adapters.py#L73-L79)), because it reads their fields as the decorator's parameters: "we validate that the data adapter class is a dataclass, and has constructor parameters that are compatible with the params to the decorator" ([apache/hamilton#150](https://github.com/apache/hamilton/issues/150), by the design's author, 2023-04). +- Adapters are registered by name in `registry.LOADER_REGISTRY` and `SAVER_REGISTRY`. One name, such as `parquet` or `csv`, maps to several classes. Hamilton chooses among them by the type the dataflow asks for, which is how one key works for pandas, polars, and other libraries ("a registry that specifies a mapping of key -> data loader classes, allowing us to polymorphically select them", [f4eafcaf](https://github.com/apache/hamilton/commit/f4eafcaf36ddbb4fffe643553aff055f53c39a5d)). Plugins register their own, as "small/lightweight extensions. Similar to the rest of the extension framework" ([apache/hamilton#151](https://github.com/apache/hamilton/issues/151)). +- Loading returns the data together with a metadata dict ([`DataLoader.load_data`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/io/data_adapters.py#L139) returns `tuple[type, dict[str, Any]]`), so what was read and written can be tracked. +- The same adapters back every entry point: the `@load_from` and `@save_to` decorators, the `from_` and `to` builders, `Driver.materialize()`, and `Builder.with_materializers()`. `materialize()` adds loader and saver nodes to the graph and executes them, and does not perform input/output (I/O) outside the graph ([134b7ecc](https://github.com/apache/hamilton/commit/134b7ecc2623bc40ff512e14fe78d380fc644250): "This manipulates the DAG and calls the materialization node"); the joining of results the spec asked for is the materializer's `combine` result builder. `from_` came later, from [#311](https://github.com/apache/hamilton/issues/311) ("`save_to` is to `materialize` as `load_from` is to `inject`"). +- The `@dataloader` and `@datasaver` decorators, added in 2024 ([#983](https://github.com/apache/hamilton/pull/983)) to have "inline loaders / savers and to expose metadata for them", are a separate, lighter path that does not use the registry. A user-written node function returns its data together with the same kind of metadata ([`docs/concepts/materialization.rst:79`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/docs/concepts/materialization.rst#L79)). + +## Consequences + +- Adding a data format means writing one dataclass and registering it, usually in a plugin extension (2611-05). It then works with every entry point. #235 states that goal: "Ideally we'd make it so if you add a new `DataAdapter` you could use it as a materializer." +- Loaders and savers are nodes, so lifecycle hooks (2611-07), visualization, and lineage see them like any other node. The documentation lists the cost for materializers added at execution time: they are visible with `Driver.visualize_materialization()` but cannot otherwise be introspected, and `Driver.materialize()` has a different call signature from `execute()` ([line 109](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/docs/concepts/materialization.rst#L109)). Materializers given to the `Builder` avoid that cost, and the documentation now calls them "typically preferrable" ([line 114](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/docs/concepts/materialization.rst#L114)). +- A loader's `applicable_types()` is part of its contract. When several classes under one name apply to the requested type, the most recently registered one wins with a logged warning ("the *last* in the list that applies", #150), and loaders that accept `Any` come last ([`resolve_adapter_class`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/function_modifiers/adapters.py#L119-L148)). That ordering was a compatibility choice: on [#536](https://github.com/apache/hamilton/pull/536) the author wrote that "The 'proper' solution would be to change custom_subclass_check, but that has the possibility of breaking some cases of materialization". Registration order is therefore observable behavior. +- Materializers are generated dynamically from the saver registry, so adapters registered at runtime become available, and import order does not matter ([#523](https://github.com/apache/hamilton/pull/523), fixing [#521](https://github.com/apache/hamilton/issues/521); its author pointed to code completion in a notebook as a way to discover the available materializers). + +## Alternatives considered + +From the #235 issue body ("Describe alternatives you've considered"), for where ad-hoc materialization happens: + +- **At Driver construction time.** Listed and not chosen in 2023; the issue gives no reason. It was adopted in 2024 as `Builder.with_materializers()` ([#877](https://github.com/apache/hamilton/issues/877), built in [#911](https://github.com/apache/hamilton/pull/911)), alongside `Driver.materialize()`. +- **With `@save_to` only.** Already possible, but "often materialization is more of an ad-hoc operation", so a call on an existing Driver was added. +- **External to the Driver** (user code does the I/O). Already possible ("also doable") and not chosen. The issue gives no reason. Documentation written in 2024 ([`materialization.rst` at e00f1448](https://github.com/apache/hamilton/blob/e00f1448164d4d2923815d732caeede003d44f4a/docs/concepts/materialization.rst#L78-L79)) names the cost: it would "Lose the Hamilton benefits of having data loading and saving part of the dataflow (visualize, lifecycle hook, etc.)" and "Adds complexity to the Driver code". diff --git a/writeups/adr/2611-07-lifecycle-adapters.md b/writeups/adr/2611-07-lifecycle-adapters.md new file mode 100644 index 000000000..ff68bc497 --- /dev/null +++ b/writeups/adr/2611-07-lifecycle-adapters.md @@ -0,0 +1,54 @@ + +# 2611-07. Lifecycle adapters replace graph adapters and result builders as the execution extension point + +**Status:** Reconstructed from codebase (decided 2023-12 in [apache/hamilton#592](https://github.com/apache/hamilton/pull/592) / commit [b7e74b76](https://github.com/apache/hamilton/commit/b7e74b76938564b6a40e9a91b865a571617f011f), tracked in umbrella issue [apache/hamilton#605](https://github.com/apache/hamilton/issues/605)) + +## Context + +Before this change, execution was customized through `HamiltonGraphAdapter` and result builders. `HamiltonGraphAdapter` arrived in 2022-02 (commit [2b610869](https://github.com/apache/hamilton/commit/2b6108694f186b5421b65b6e4bf032b231bc64c5), in [stitchfix/hamilton#47](https://github.com/stitchfix/hamilton/pull/47)), whose author "decided to have it extend ResultMixin to simplify things", tying execution and result building into one class. The commit says the lifecycle work was "meant to: 1. Replace GraphAdapters 2. Replace ResultBuilders 3. Add new capabilities to pre/post node execution 4. Consolidate some of the extra things we'd considered adding/additional features we have (data quality potentially, logging to OpenTel, debugging, etc...)". + +The post that announced it, "[Customizing Hamilton's execution with lifecycle hooks](https://blog.dagworks.io/p/customizing-hamiltons-execution-with)" (2024-01), linked from [#605's closing comment](https://github.com/apache/hamilton/issues/605#issuecomment-1898837617), gives the aim: "Rather than stretching ourselves thin by reinventing the execution, auditing, telemetry, and validation wheels for every conceivable context, we have decided to keep Hamilton itself simple and add the ability to build/integrate any of these capabilities yourself!" + +This record was reconstructed from commit [b7e74b76](https://github.com/apache/hamilton/commit/b7e74b76938564b6a40e9a91b865a571617f011f) (2023-12-22, "Implements Lifecycle Customizations") and the pull request that carried it, [#592](https://github.com/apache/hamilton/pull/592) (opened 2023-12-17, merged 2023-12-26); from umbrella issue [#605](https://github.com/apache/hamilton/issues/605), opened afterward with the commit's message; from the module docstring of [`hamilton/lifecycle/base.py:18-43`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/lifecycle/base.py#L18-L43); and from the post the issue's closing comment links. The commit message has a section "What decisions have been made that might impact design?", so most of the reasoning is recovered. #592 was written by Elijah ben Izzy; Stefan Krawczyk approved it, writing "I have run the branch locally and it seems to not introduce any regressions" ([review](https://github.com/apache/hamilton/pull/592#pullrequestreview-1796091436)). + +## Decision + +- Execution is customized by lifecycle adapters of three kinds. *Hooks* augment execution. Several hooks of the same kind can be layered and all run, for example `NodeExecutionHook` and `GraphExecutionHook`. *Methods* replace a step of Hamilton's execution, for example `NodeExecutionMethod` and `ResultBuilder`, and "can not (currently) be layered" ([`hamilton/lifecycle/base.py:24-25`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/lifecycle/base.py#L24-L25)). *Validators* check the graph or nodes and were added in [#633](https://github.com/apache/hamilton/pull/633). +- Each stage is a single-method class with a unique name, registered through the [`lifecycle` decorator](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/lifecycle/base.py#L184). Stages do not share a common base class or metaclass. The commit gives the reason: "This allows us to be flexible with the function structure, and, more importantly, allows for easier multiple inheritance to ensure MRO is simple to find." MRO is Python's method resolution order. One user class can implement several stages by inheriting several application programming interface (API) classes. +- There are two layers. Internal base classes in `hamilton/lifecycle/base.py` are "*not* a public facing API", and the post says they "may not necessarily be stable between minor/patch versions". Users extend the classes in `hamilton/lifecycle/api.py`, whose methods take keyword arguments plus `**future_kwargs` "to ensure we can add (but not subtract) parameters and nothing will break". +- Each stage can have a sync and an async version. +- The package was first called `customization`; it became `lifecycle` at the reviewer's suggestion, the author agreeing it was "more consistent and descriptive enough" ([review comment](https://github.com/apache/hamilton/pull/592#discussion_r1435888832)). +- The Driver groups the adapters it receives into a [`LifecycleAdapterSet`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/lifecycle/base.py#L880) ([`hamilton/driver.py:336`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/driver.py#L336)) and calls stages through it. +- Legacy APIs remain as adapters built on the new ones. [`lifecycle.api.GraphAdapter`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/lifecycle/api.py#L121-L127) composes the node-execution, result, input-validation, and edge-type-check methods, and [`hamilton.base.HamiltonGraphAdapter`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/base.py#L408) extends it. [`LegacyResultMixin`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/lifecycle/api.py#L106) keeps the old static `build_result`, and [`hamilton.base.ResultMixin`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/base.py#L42) extends it. + +## Consequences + +- Later execution features ship as lifecycle adapters: tracing and tracking (`DDOGTracer`, `MLFlowTracker`), `OpenLineageAdapter`, `ProgressBar`, `SlackNotifierHook`, `PDBDebugger`, and others ([`docs/reference/lifecycle-hooks/`](https://github.com/apache/hamilton/tree/ef88b1e96140a01f3d72967316c2335f8b62a067/docs/reference/lifecycle-hooks)). Caching is one too: the first cache adapter ([#766](https://github.com/apache/hamilton/pull/766)) is now deprecated, and its replacement, [`HamiltonCacheAdapter`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/caching/adapter.py#L194-L196) ([#1104](https://github.com/apache/hamilton/pull/1104)), is built on the lifecycle base classes. +- An adapter set can hold any number of hooks but at most one implementation of each method ([`base.py:963`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/lifecycle/base.py#L963)). The cost is concrete: caching uses the single `do_node_execute()` method, so it "is currently incompatible with other adapters leveraging it", such as `PDBDebugger` ([`docs/reference/caching/caching-logic.rst:63`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/docs/reference/caching/caching-logic.rst#L63)). +- Existing `GraphAdapter` and `ResultMixin` subclasses keep working, but the two systems coexist. Code in `hamilton.base` still carries the legacy class hierarchy. [b7e74b76](https://github.com/apache/hamilton/commit/b7e74b76938564b6a40e9a91b865a571617f011f) listed a "3. Deprecation" step for graph adapters that has not happened; the documentation now presents them as "special cases" of lifecycle hooks that "implement multiple lifecycle customizations in a single place" ([`docs/reference/graph-adapters/index.rst:6`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/docs/reference/graph-adapters/index.rst#L6)). +- #592's author also meant to retire `Driver.raw_execute`: "we don't want people to use raw_execute anymore, just the driver builder" ([review comment](https://github.com/apache/hamilton/pull/592#discussion_r1435775253)). It is still in [`hamilton/driver.py`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/driver.py#L613). +- Adding a stage means four steps listed in the `base.py` docstring: a single-method class, the `lifecycle` decorator, an entry in the `LifecycleAdapter` type, and a call at the right point in execution. They are not always all taken: `BaseDoRemoteExecute` ([#1103](https://github.com/apache/hamilton/pull/1103)) is missing from the [`LifecycleAdapter` type](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/lifecycle/base.py#L850-L877). + +## Alternatives considered + +- **One base class or metaclass for all lifecycle stages.** Rejected in favor of registration, for flexible function signatures and simple multiple inheritance ([b7e74b76](https://github.com/apache/hamilton/commit/b7e74b76938564b6a40e9a91b865a571617f011f)). +- **Keep extending `GraphAdapter` and result builders.** The change was explicitly meant to replace them. The commit notes that async "never worked with graph adapters". It does not otherwise say why extending them was rejected. +- **One abstract class per hook with `before` and `after` methods.** Prototyped in 2023-07 as `hamilton/execution/hooks.py_backup` (commit [d410e126](https://github.com/apache/hamilton/commit/d410e12605f078903198d88204925a7073b3633f), in [#220](https://github.com/apache/hamilton/pull/220)) and deleted from that pull request as "not needed now" ([review comment](https://github.com/apache/hamilton/pull/220#discussion_r1272709517)). +- **Hooks that wrap functions when the graph is built.** Raised in review on #592 and deferred: the author answered that decorators already act as such hooks, and that he wanted "to push that off till later" ([review comment](https://github.com/apache/hamilton/pull/592#discussion_r1435889364)). diff --git a/writeups/adr/2611-08-apache-hamilton-distribution.md b/writeups/adr/2611-08-apache-hamilton-distribution.md new file mode 100644 index 000000000..471754e0d --- /dev/null +++ b/writeups/adr/2611-08-apache-hamilton-distribution.md @@ -0,0 +1,50 @@ + +# 2611-08. Packages are distributed as `apache-hamilton*`; the import name stays `hamilton`, and the `sf-hamilton*` names become redirects + +**Status:** Reconstructed from codebase (decided 2025-10 on the 1.89.0 release vote, where a binding −1 cited the Incubator's naming rule; carried out from 2026-02 to 2026-05 in [apache/hamilton#1408](https://github.com/apache/hamilton/pull/1408) / commit [7e2ffb84](https://github.com/apache/hamilton/commit/7e2ffb841a812c258cca0c53b01dd14925d3b3fa), [#1488](https://github.com/apache/hamilton/pull/1488), [#1539](https://github.com/apache/hamilton/pull/1539) and [#1588](https://github.com/apache/hamilton/pull/1588)) + +## Context + +Hamilton was published to the Python Package Index (PyPI) as `sf-hamilton` (with `sf-hamilton-sdk`, `-ui`, `-lsp`, and `-contrib`). It entered the Apache Software Foundation (ASF) Incubator in 2025. Its [incubation proposal](https://cwiki.apache.org/confluence/display/INCUBATOR/HamiltonProposal) gives the aims as establishing an open standard, community-driven development, and vendor neutrality: "Many existing graph execution and orchestration tools are tied to commercial platforms." It was relicensed to Apache 2.0 "because we're an apache project now" ([#1335](https://github.com/apache/hamilton/pull/1335), 2025-06). Releases are voted on as source releases. + +On the project's own vote for 1.89.0, PJ Fanning wrote that "The source release which is required must have 'incubating' in the name. If you want to also release an sf-hamilton - then you can have 2 tar gz files." ([email](https://lists.apache.org/thread/94y9wso9135hmm3qcwp7rg6hok20rc0d), 2025-09-14). On the general@ vote for 1.89.0-incubating, Justin Mclean voted −1 (binding): "For distribution on PyPI, please see [1]. It appears that you are not following this, particularly in the naming of the artifact" ([email](https://lists.apache.org/thread/8nrk1jtmskk3sn2jnxnpqp6tfvkzr832), 2025-10-06). [1] is the Incubator's [distribution guide](https://incubator.apache.org/guides/distribution.html#pypi): "Artifacts need to be placed in https://pypi.org/project/apache-". Stefan Krawczyk answered that "We need those artifact names to conform" ([email](https://lists.apache.org/thread/gq9y4wvpvbnxgk72gntsspqoq0dpld0l)). The vote passed, and his result email proposed "that we not move the sf_hamitlon* artifacts over to the main Apache Release since everything under there should follow Apache naming conventions", adding "Next time we won't vote on them" ([email](https://lists.apache.org/thread/gkf95nkf5otyj5v95h63yhzplo8tp9v6), 2025-10-10). `apache-hamilton` 1.89.0 [reached PyPI](https://pypi.org/project/apache-hamilton/1.89.0/) as a source distribution on 2025-10-12, the day after `sf-hamilton` 1.89.0. + +The repository followed. [7e2ffb84](https://github.com/apache/hamilton/commit/7e2ffb841a812c258cca0c53b01dd14925d3b3fa) gives the reason for renaming the build itself: "This will make it simpler to release things. Since we vote on source releases -- this will make it simpler to do so rather than much post-processing." #1408 adds: "We should only create apache-hamilton artifacts for the source release vote. We can post-process for upload to pypi." Its two approvals date from 2025-10; the rename commit was pushed and merged by its author on 2026-02-21. #1488 renamed and converted the sub-packages "for Apache incubation compliance", with flit "because @potiuk suggested it, and just making this standard across the repo" ([review comment](https://github.com/apache/hamilton/pull/1488#discussion_r2850470985)). Jarek Potiuk had recommended flit on the 1.89.0 vote: it "has 0 dependencies", generates no `.egg-info` artifacts, and spares the project setuptools' "stability" issues ([email](https://lists.apache.org/thread/ccfw895jz9f2gv6pbmfcm9fz7fmcv2xo)). #1488's approval says it was "discussed at length offline with @skrawcz" ([review](https://github.com/apache/hamilton/pull/1488#pullrequestreview-3900538376)). + +This record was reconstructed from the release-vote threads on general@incubator.apache.org named below, commit [7e2ffb84](https://github.com/apache/hamilton/commit/7e2ffb841a812c258cca0c53b01dd14925d3b3fa) ("Updates package to be apache-hamilton") and pull request [#1408](https://github.com/apache/hamilton/pull/1408), [#1488](https://github.com/apache/hamilton/pull/1488) (the sub-packages), [#1539](https://github.com/apache/hamilton/pull/1539) and [#1588](https://github.com/apache/hamilton/pull/1588) (the redirects), [`pyproject.toml`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/pyproject.toml#L314-L315), and [`sf-hamilton-redirect/`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/sf-hamilton-redirect). + +## Decision + +- The distributions are `apache-hamilton`, `apache-hamilton-sdk`, `apache-hamilton-ui`, `apache-hamilton-lsp`, and `apache-hamilton-contrib`, all built with flit. +- The import package is unchanged: `[tool.flit.module] name = "hamilton"` ([`pyproject.toml:314-315`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/pyproject.toml#L314-L315)). User code keeps `import hamilton`. +- Each old name is published as a redirect package ([`sf-hamilton-redirect/`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/sf-hamilton-redirect), `sf-hamilton-{sdk,ui,lsp,contrib}-redirect/`). It has no code of its own and depends on the matching `apache-hamilton*` package at the same version ([`sf-hamilton-redirect/pyproject.toml.template:32`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/sf-hamilton-redirect/pyproject.toml.template#L32)), forwarding its extras "so existing sf-hamilton users get the new release seamlessly" ([cde95ef6](https://github.com/apache/hamilton/commit/cde95ef61bcdebe5c65a6ca875c78b01c77420d4), #1539). The redirects are built with setuptools: "Redirect packages have no Python source code", and flit needs a module to build ([review comment](https://github.com/apache/hamilton/pull/1588#discussion_r3293319716)). Its README tells users to "Replace `sf-hamilton` with `apache-hamilton` in your dependencies" ([`README.md:27`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/sf-hamilton-redirect/README.md#L27)). + +## Consequences + +- Requirements that name `sf-hamilton` keep installing, and new releases of it pull in `apache-hamilton`, so dependents can migrate gradually. The first redirect release was `sf-hamilton` 1.90.0 (2026-04-25). +- The redirect templates render their extras and Python bound at build time from the packages' own `pyproject.toml` ([#1805](https://github.com/apache/hamilton/pull/1805)). Copied by hand, they had drifted: the main template lacked the `performance` extra, the sub-package templates declared no extras, and all five still accepted Python 3.10 after [#1798](https://github.com/apache/hamilton/pull/1798) raised every `apache-hamilton*` package to `requires-python = ">=3.11, <4"`. +- New sub-packages are named `apache-hamilton-`, and their artifacts are what the release vote covers. Each package is voted on separately: when the four sub-packages came up in one vote, PJ Fanning asked to "stop this vote and start 4 new separate votes" ([email](https://lists.apache.org/thread/mqc6gy3qmngx62x1f7wf2bhnf97f8rz6), 2026-05-26). +- The old names still draw the Incubator's scrutiny: in 2026-05 Justin Mclean noted that "The sf-hamilton package does not carry the ALv2 license field or an Incubator disclaimer" ([email](https://lists.apache.org/thread/59h0yhb993sx3v6cvjt0pgpqkxbbq7wz)). +- Source files carry the ASF license header, enforced by the `insert-license` hooks in [`.pre-commit-config.yaml`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/.pre-commit-config.yaml#L50-L57) (`docs/` and `examples/` are excluded). + +## Alternatives considered + +- **Keep the `sf-hamilton` distribution names.** Rejected on the 1.89.0 vote: the binding −1 named the artifact names, and the result email moved the `sf_hamilton` artifacts out of the Apache release. +- **Vote on `sf-hamilton` artifacts and rename the source release afterwards.** The release script did this for 1.89.0, copying the tarball to `apache-hamilton--incubating.tar.gz` ([a2217ff6](https://github.com/apache/hamilton/commit/a2217ff695820d693437d8cde590708a917c7d6a), [#1378](https://github.com/apache/hamilton/pull/1378)). On the vote Stefan Krawczyk allowed that "If we should just post-process the source release then that's fine. But I thought this approach is clearer and more transparent" ([email](https://lists.apache.org/thread/gq9y4wvpvbnxgk72gntsspqoq0dpld0l)), and [7e2ffb84](https://github.com/apache/hamilton/commit/7e2ffb841a812c258cca0c53b01dd14925d3b3fa) chose to build `apache-hamilton` artifacts directly to avoid "much post-processing". diff --git a/writeups/adr/2611-09-no-usage-telemetry.md b/writeups/adr/2611-09-no-usage-telemetry.md new file mode 100644 index 000000000..9f7ba4938 --- /dev/null +++ b/writeups/adr/2611-09-no-usage-telemetry.md @@ -0,0 +1,47 @@ + +# 2611-09. Released Hamilton artifacts do not phone home + +**Status:** Reconstructed from codebase (decided 2026-03 in [apache/hamilton#1512](https://github.com/apache/hamilton/pull/1512) / commit [45673b02](https://github.com/apache/hamilton/commit/45673b020f84f23eb982d0a563fe057cd971aeab), released in 1.90.0) + +## Context + +Opt-out usage telemetry was added in 2022-12 (commit [0dcd52a7](https://github.com/apache/hamilton/commit/0dcd52a71c32c3b4b43245c2603efa93b645ae29)). By default, the library sent anonymous usage events to PostHog when a Driver was created and when `execute()` finished. The events covered the operating system and Python version, a persistent universally unique identifier (UUID) kept in `~/.hamilton.conf`, Hamilton-only stack traces, and the decorators, adapters, and result builders in use. Users could turn it off with `telemetry.disable_telemetry()`, the `telemetry_enabled` key in `~/.hamilton.conf`, or `HAMILTON_TELEMETRY_ENABLED=false`. The message for [0dcd52a7](https://github.com/apache/hamilton/commit/0dcd52a71c32c3b4b43245c2603efa93b645ae29) gives the purpose, "to help us improve Hamilton and know where to apply development efforts", and the promise: "No user data or potentially sensitive information is or ever will be collected." + +After the project entered the incubator of the Apache Software Foundation (ASF), #1512 gave the reason for removing it: "ASF policy does not permit phone-home telemetry in released artifacts." The pull request links no policy text, and no discussion of the removal on the project's mailing lists was found. It was opened and merged on 2026-03-08 by Stefan Krawczyk, approved by mentor Jarek Potiuk and by Dev-iL after a review that asked for deprecation ([review](https://github.com/apache/hamilton/pull/1512#pullrequestreview-3911170080)). The 1.90.0 release vote listed "Remove usage telemetry (PostHog) (#1512)" and passed the project's vote ([thread](https://lists.apache.org/thread/flo5w8347vr2yo7c4ssv1ndyd2c47dg2)) and the Incubator's ([thread](https://lists.apache.org/thread/po2w9xsbxj2d0j99fhml0spg7kh919tm)) with no comment on the change. + +This record was reconstructed from pull request [apache/hamilton#1512](https://github.com/apache/hamilton/pull/1512) (body and reviews), commit [0dcd52a7](https://github.com/apache/hamilton/commit/0dcd52a71c32c3b4b43245c2603efa93b645ae29), [`hamilton/telemetry.py`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/telemetry.py#L29-L50), and the 1.90.0 release-vote threads. + +## Decision + +- All PostHog event tracking is removed from the library, the command-line interface (CLI), the experiments server, `contrib`, dataflows, and the user interface (UI) backend's tracking-server app ([45673b02](https://github.com/apache/hamilton/commit/45673b020f84f23eb982d0a563fe057cd971aeab) touches 25 files). +- [`hamilton/telemetry.py`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/hamilton/telemetry.py#L29-L50) remains as a no-op stub, so existing calls to `telemetry.disable_telemetry()` keep working. `is_telemetry_enabled()` always returns `False`. +- The review on #1512 asked to "mark these methods as deprecated (via decorator, or raising a warning), and remove all traces in an upcoming major release", so both stub functions use `@deprecated(warn_starting=(1, 89, 0), fail_starting=(2, 0, 0))` (see 2611-04). + +## Consequences + +- No feature may add phone-home telemetry to a released artifact, as #1512 states the policy. The project loses the usage data it once used to decide where to apply development effort. +- The stub first shipped in 1.90.0; 1.89.0 still sent telemetry. Its `warn_starting` of 1.89.0 has passed, so calling it warns: through a logged warning in 1.90.0, and through a `FutureWarning` from the first release that carries [#1804](https://github.com/apache/hamilton/pull/1804) (see 2611-04). It raises `DeprecationError` from 2.0.0 on. +- The stub keeps only the two public functions, so code that imported anything else from `hamilton.telemetry` broke: `hamilton ui` failed with "cannot import name 'API_KEY' from 'hamilton.telemetry'" until the UI package was fixed in its next release ([#1649](https://github.com/apache/hamilton/issues/1649#issuecomment-5771274282)). +- PostHog code remains outside the core library: `ui/frontend` wraps the app in `PostHogProvider` when it is built with `VITE_USE_POSTHOG=true` ([`ui/frontend/src/index.tsx:64`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/ui/frontend/src/index.tsx#L64)) and identifies the user to it ([`App.tsx:111`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/ui/frontend/src/App.tsx#L111)); `ui/sdk` still lists `posthog` as a dependency ([`pyproject.toml:52`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/ui/sdk/pyproject.toml#L52), [`requirements.txt:7`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/ui/sdk/requirements.txt#L7)), though no source file of the software development kit (SDK) imports it; and the `contrib` documentation site loads a PostHog script ([`docusaurus.config.js:97`](https://github.com/apache/hamilton/blob/ef88b1e96140a01f3d72967316c2335f8b62a067/contrib/docs/docusaurus.config.js#L97)). The UI's 0.0.18 release notes list "Removed PostHog telemetry" ([vote](https://lists.apache.org/thread/d4fzmqq4jbsx5m3krdp1qbr0wh9j131s)), but the frontend code above remains behind its build flag. Whether these fall under the same policy was not settled in #1512. +- Run tracking that a user points at their own Hamilton UI (`HamiltonTracker` in the SDK) is user-configured data collection, not project telemetry, and this record does not cover it. + +## Alternatives considered + +- **Delete `hamilton/telemetry.py` outright.** Rejected in favor of a stub, so that "downstream code that calls `telemetry.disable_telemetry()` continues to work without changes" (#1512). diff --git a/writeups/adr/README.md b/writeups/adr/README.md index 8dc0dcf0f..62826a83e 100644 --- a/writeups/adr/README.md +++ b/writeups/adr/README.md @@ -24,8 +24,7 @@ it costs, and the alternatives that were rejected. The records outlive the commi threads that produced them, so a later change can tell a deliberate choice from an accident. - File name: `YYMM-NN-short-title.md`, numbered in order. Numbers are never reused. -- Status: `Accepted`, `Superseded by YYMM-NN`, or `Deprecated`. An accepted record is not edited to - change its decision; write a new record that supersedes it. +- Status: `Accepted`, `Superseded by YYMM-NN`, `Deprecated`, or `Reconstructed from codebase`. An accepted record is not edited to change its decision; write a new record that supersedes it. A record marked `Reconstructed from codebase` was written after the fact, from the code and the history behind it, for a decision the code already embodies: its Status line names where that decision was made, and the status claims no acceptance of the record itself. A reconstructed record is changed the same way as an accepted one. - Sections: Status, Context, Decision, Consequences, Alternatives considered. | ADR | Title | @@ -35,3 +34,11 @@ threads that produced them, so a later change can tell a deliberate choice from | [2609-03](2609-03-resolving-sql-writes-and-statements.md) | How datasource-mode lineage resolves what a SQL write names | | [2609-04](2609-04-sqlite-attached-databases.md) | SQLite attached databases are named by their own file | | [2611-01](2611-01-polars-database-sql-metadata.md) | Polars database I/O records `sql_metadata` beside `file_metadata`; the pre-1.0 integration stays legacy-only | +| [2611-02](2611-02-functions-define-nodes.md) | A function defines a node, and its parameter names are its dependencies | +| [2611-03](2611-03-function-modifiers-fixed-lifecycle.md) | Function modifiers annotate a function and resolve in a fixed lifecycle at graph-build time | +| [2611-04](2611-04-version-gated-deprecations.md) | Deprecations name the major release that removes them, through a warn/fail version contract | +| [2611-05](2611-05-extension-registry-autoload.md) | Library integrations load through a dependency-free registry, autoloaded unless the user opts out | +| [2611-06](2611-06-data-adapters-and-materialization.md) | Data I/O is done by registered loader and saver classes that become nodes in the DAG | +| [2611-07](2611-07-lifecycle-adapters.md) | Lifecycle adapters replace graph adapters and result builders as the execution extension point | +| [2611-08](2611-08-apache-hamilton-distribution.md) | Packages are distributed as `apache-hamilton*`; the import name stays `hamilton`, and the `sf-hamilton*` names become redirects | +| [2611-09](2611-09-no-usage-telemetry.md) | Released Hamilton artifacts do not phone home | diff --git a/writeups/developer_setup.md b/writeups/developer_setup.md index 711a6ce7d..28dedfb9c 100644 --- a/writeups/developer_setup.md +++ b/writeups/developer_setup.md @@ -39,20 +39,13 @@ cd ./hamilton git remote add upstream https://github.com/apache/hamilton.git ``` -Install the project's dependencies in your preferred method for managing python dependencies. For example, run the following to use `venv`. +Install the project's dependencies with [uv](https://docs.astral.sh/uv/), which is what continuous integration (CI) uses. The `dev` and `test` dependency groups in `pyproject.toml` hold the linting and test tools. They are dependency groups, not extras, so `pip install '.[dev,test]'` does not install them. ```shell -python -m venv ./venv -. ./venv/bin/activate - -pip install .[dev,test] +uv sync --group test ``` -Please be informed to use escape characters or wrap the argument in quote as shown in the command below since the "[]" or "." may be treated as special characters leading to failure in finding matching files. - -```shell -pip install '.[dev,test]' -``` +This creates `.venv` with Hamilton, the `test` group, and the `dev` group, which uv installs by default. Activate it with `. .venv/bin/activate`, or prefix the commands below with `uv run`. Set up `prek`, which will run some lightweight formatting and linting tasks on every commit. @@ -96,61 +89,25 @@ Navigate to https://github.com/apache/hamilton/pulls and open a pull request. ## How to run unit tests -You need to have installed the `requirements-test.txt` dependencies into the environment you're running for this to work. You can run tests two ways: - -1. Through pycharm/command line. -2. Using circle ci locally. The config for this lives in `.circleci/config.yml` which also shows commands to run tests -from the command line. - -### Running tests in Docker - -The most reliable way to run `hamilton`'s unit tests is to simulate its continuous integration (CI) environment in docker. +CI runs on GitHub Actions, and [`.github/workflows/hamilton-main.yml`](../.github/workflows/hamilton-main.yml) holds its commands. Running the same commands locally is the most reliable way to reproduce a CI result. -`hamilton`'s CI logic is defined in shell scripts, whose behavior changes based on environment variable `TASK`. - -The following values for `TASK` are recognized: - -* `async` = unit tests using the async driver -* `dask` = unit tests using the `dask` adapter -* `integrations` = tests on integrations with other frameworks -* `prek` = static analysis (i.e. linting) -* `pyspark` = unit tests using the `spark` adapter -* `ray` = unit tests using the `ray` adapter -* `tests` = core unit tests with minimal requirements - -Choose a Python version and task. +Install the `test` dependency group, then run the core unit tests as CI's "Test hamilton main package" step does: ```shell -PYTHON_VERSION='3.11' -TASK=tests +uv sync --group test +uv run pytest tests/ --ignore tests/integrations ``` -Then run the tests for that combination in a container. - -```shell -docker run \ - --rm \ - --entrypoint="" \ - -v "$(pwd)":/opt/testing \ - --workdir /opt/testing \ - --env TASK=${TASK} \ - -it circleci/python:${PYTHON_VERSION} \ - /bin/bash -c '.ci/setup.sh && .ci/test.sh' -``` +CI also passes `--cov=hamilton` to measure coverage. A separate step runs `tests/integrations`, with the `pandera` extra and `tests/integrations/pandera/requirements.txt` installed. Other steps test single integrations, such as the plugin tests under `plugin_tests/` (for example `plugin_tests/h_dask`), the Model Context Protocol (MCP) tests, and the OpenLineage tests. Each step installs what it needs on top of the `test` group, for example `uv sync --group test --extra dask`. Read the step in the workflow for its exact commands. -### Using pycharm to execute & debug unit tests +Some tests need more than Python packages: -You can debug and execute unit tests in pycharm easily. To set it up, you just hit `Edit configurations` and then -add New > Python Tests > pytest. You then want to specify the `tests/` folder under `Script path`, and ensure the -python environment executing it is the appropriate one with all the dependencies installed. If you add `-v` to the -additional arguments part, you'll then get verbose diffs if any tests fail. +- The graph-rendering tests in `tests/test_graph.py` run Graphviz's `dot` program. Install Graphviz with your system's package manager (CI runs `apt-get install graphviz` on Linux). +- Some tests in `tests/plugins/test_h_openlineage.py` and `tests/plugins/test_pandas_extensions.py` need a PostgreSQL server named by the `HAMILTON_TEST_POSTGRES_URL` environment variable. They are skipped when it is not set (`tests/conftest.py`). -### Using circle ci locally +### Using PyCharm to execute & debug unit tests -You need to install the circleci command line tooling for this to work. -See https://circleci.com/docs/local-cli/ for details. -Once you have installed it you just need to run `circleci local execute` from the root directory and it'll run the entire suite of tests -that are setup to run each time you push a commit to a branch in github. +You can debug and execute unit tests in PyCharm. To set it up, hit `Edit configurations` and then add New > Python Tests > pytest. Specify the `tests/` folder under `Script path`, and make sure the Python interpreter is the one in `.venv`. If you add `-v` to the additional arguments, you'll get verbose diffs if any tests fail. # Pushing to pypi These are the steps to push to pypi. This is taken from the [python packaging tutorial](https://packaging.python.org/tutorials/packaging-projects/#generating-distribution-archives).