Repository navigation
fix: Emit one position per axis in BBI tileset info - #11
Merged
Merged
Conversation
`BBITileset._info_for` built its info without passing `ndim` and then overwrote `max_pos` with a one-element list, so `BBIInteraction2DTileset` advertised `min_pos: [0]` / `max_pos: [N]` despite being two-dimensional. A 2D track takes its y extent from `max_pos[1]`, so a `2d-rectangle-domains` track backed by a bigInteract rendered nothing. The tile payloads were already correct -- record for record identical to what `BedpeTileset` serves for the same interactions. Pass `self.ndim` through to `TilesetInfo.quadtree` and pad `max_pos` per axis. The 1D tilesets are unaffected.
Contributor
|
I'll add tests before this merges. |
Assigning to a model field after construction only works while the model is mutable. The branch restoring the dropped interface guards freezes TilesetInfo, and this assignment raises a pydantic validation error there -- from info(), which tiles() calls on its first line, so it would take every BBI tileset dark rather than only the two-dimensional one this function was just fixed for. Copying produces the same info here, at both arities, so the change is invisible until the two branches meet and removes the failure when they do. Once that branch lands this should become TilesetInfo.with_, which validates the update and drops the cached coordinate system where model_copy does neither.
The module had no tests at all, so neither the arity fix nor the one-dimensional behavior it must not disturb was covered by anything. Five cases, three of them parametrized across the concrete tilesets. The two-dimensional tileset is asserted to publish two entries, the three one-dimensional ones exactly one, and every tileset as many as it declares -- the last so a type added later cannot quietly inherit the wrong shape. A fourth pins what each axis is padded to, since padding per axis with the genome length would satisfy the arity assertions while putting both axes in the wrong place. The fifth pins the type-specific info fields, which are passed as keywords beside the new arity keyword. Verified against the unfixed source: the three arity and padding cases fail there, while the one-dimensional and extras cases pass either way, which is what makes them controls rather than pins. Fixtures are synthesized with pybigtools rather than read from data, so the module runs on a checkout with no git-LFS payload.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Allows
BBIInteraction2DTilesetto work as a 2D tileset.It inherited
BBITileset._info_for, which advertisedmin_pos: [0]/max_pos: [N]despite being two-dimensional.To fix this, we pass
self.ndimthrough toTilesetInfo.quadtreeand padmax_posper axis. The 1D tilesets are unaffected.Closes #12