Symbol joining on to_dataframe (instrument_id → raw_symbol)#39
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82b75c63d9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| import DatabentoBinaryEncoding as DBN | ||
| using DatabentoBinaryEncoding: Schema, SType, Compression, Encoding, Action, Side, InstrumentClass | ||
| using DatabentoBinaryEncoding: symbol_map, symbol_for, add_symbol_column! |
There was a problem hiding this comment.
Bump DBN compat before importing new helpers
When this package is installed in a fresh environment, Project.toml still allows DatabentoBinaryEncoding = "0.1.2", but this line imports helpers that the commit message says come from the separate symbol-resolution DBN release. With the currently allowed DBN version, using DatabentoAPI can fail during module loading before any user code runs; bump the compat/version requirement in the same change that imports these names.
Useful? React with 👍 / 👎.
Records carry only the opaque numeric instrument_id; the human-readable symbol lives in metadata.mappings. to_dataframe(store; symbols=true) now joins the raw_symbol onto the frame as a :symbol column, resolved per record from store.metadata.mappings via DatabentoBinaryEncoding's new symbol helpers. The join keys on instrument_id/ts_event, follows instrument-id rolls across the query range, and yields missing for unmatched rows. It applies only when the data was fetched with stype_out = SType.INSTRUMENT_ID (the get_range default). Default is symbols=false, so existing to_dataframe calls are unaffected. Re-exports symbol_map / symbol_for / add_symbol_column! from DatabentoBinaryEncoding so streaming consumers (foreach_record, which returns the metadata) can build the lookup once and resolve symbols per record. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses Codex P1: this package now imports symbol_map / symbol_for / add_symbol_column! from DatabentoBinaryEncoding, which first exist in its 0.1.4 release. Bump the [compat] lower bound from 0.1.2 to 0.1.4 so a fresh install can't resolve an older DBN that lacks these bindings (which would make `using DatabentoAPI` fail during module load). Also drop the [`symbol_map`](@ref) / [`symbol_for`](@ref) cross-reference links from the to_dataframe docstring — those helpers are documented in the DatabentoBinaryEncoding docs, not spliced into this package's API reference, so the @refs would fail Documenter's cross_references check. Use plain code spans. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Records carry only the opaque numeric
instrument_id; the human-readable symbol lives in the responsemetadata.mappings. This adds asymbolskwarg toto_dataframethat joins theraw_symbolback onto the frame:The join keys on
instrument_id/ts_event, follows instrument-id rolls across the query range, and yieldsmissingfor unmatched rows. It applies only when data was fetched withstype_out = SType.INSTRUMENT_ID(theget_rangedefault). Default issymbols=false, so existingto_dataframecalls are unchanged.For streaming consumers (
foreach_record, which returns theDBN.Metadata), the helperssymbol_map,symbol_for, andadd_symbol_column!are re-exported so you can build the lookup once and resolve per record:Where the logic lives
Per the package's delegation boundary (this package owns wire protocols, not record/metadata layout), the actual join logic lives in DatabentoBinaryEncoding.jl (it owns
Metadata, the mapping semantics, andrecords_to_dataframe) and benefits standalone.dbnfile readers too. This PR is the thin API wrapper + re-exports.Dependency
Requires the DatabentoBinaryEncoding symbol-resolution helpers: tbeason/DatabentoBinaryEncoding.jl#38. Merge + release that first, then bump the
[compat]entry here.Tests
test/test_conversion.jlgains a "to_dataframe symbol join" testset: default-off behavior, the joined:symbolcolumn, and the re-exportedsymbol_map/symbol_forhelpers.Note (out of scope)
While testing I found that
src/store.jldefines_foreach_typed_toleranttwice (identical copy-paste), which makes the package fail clean precompilation on Julia 1.12+ (Pkg.testmasks it via fallback, so CI is green). Tracked/fixed separately to keep this PR focused.🤖 Generated with Claude Code