IPPM code has some mix-up between mutating and non-mutating function calls #367
Assignees
Labels
👷 design standards
IPPM generation
⚙️ refactor
Changes which relate to code structure rather than functionality
Comments
caiw
added
👷 design standards
⚙️ refactor
14 tasks
caiw
added a commit
that referenced
this issue
Sep 13, 2024
…(see #367). It'll need sorting at some point but for now it's ok.
neukym
added a commit
that referenced
this issue
Sep 13, 2024
* Add edge labels [replay 4810560] * shift edge paths to make nicer curves [replay d318920] * Remove unused code * Fix test mocking * Rename `kymata.ippm.{plotter => plot}` [replay ca61bc9 in part] * The IPPMPlotter was just a housing to a set of static methods so didn't need to be instantiated (i.e. it has no state). The more Pythonic way to do this imo is to use bare functions rather than static methods, especially when there's only one public function. [replay rest of ca61bc9] * Rename `kymata.ippm.{builder => build}` [replay f68c168 in part] * Also convert IPPMBuilder to a set of bare functions rather than static methods on a static class. [replay rest of f68c168] * The rest of f68c168 can't be replayed on the new code, so rewind * Reorganise docs and remove unreferenced images [replay e9040d3] * Clean up type annotation Rename `function_recall()` to `transform_recall()` as per latest paper draft [replay ad9b34f] * Fix merge errors * Rename some things * Lint * Fix test mocking * Add arrowheads [replay f8b80bf] * Nodes are little hexagons [replay 27d2048] * [replay dea5870] * minor changes to arrowhead dimensions [replay 7967d85] * Use constants for left/right hemispheres, rather than repeated constants [partial replay of 3310c57] * Rename `IPPMHexel` to `IPPMSpike` * Merge error * Remove outdated notebooks * Typo * Notebook fixes * Optional titles * Fix mocking * Rename "hexels" to "spikes" * Gracefully handle transforms with no significant spikes + update hexel -> spike naming in demo * Fix test * Renaming in test * Pull out string constants * Make interface abstract * Type-alias `SpikeDict = dict[str, IPPMSpike]`, and move stem plot to `ippm.plot` * Type alias `dict[str, IPPMNode]` to `NodeDict` * Type alias `dict[str, list[str]]` to `TransformHierarchy`. And update `list`, `tuple` and `dict` type hints to modern python * Calling code shouldn't need to know to call `.build_graph_dict()` after constructing `IPPMBuilder`. Just do it automatically and cache it (which is what happened anyway) * `tuple[float, float]` → `class Pairing(NamedTuple)` plus extra type hints * This function sorts in place so don't need to return and assign * Move more plotting functions to `ippm.plot` * Missed some magic strings * And some more magic strings * Move `transform_recall` and `causality_violation` to `ippm.evaluation` * Make public * fixes #360 * Check for singular case and remove anomaly detection in GMM * Shuffle + Merge Hemispheres * Fix ruff error * Update matplotlib version * Be a bit more explicit about class names * Move dependent method below in __init__ order * Fix some `.fit()` errors * There's some mix-up between mutating and non-mutating function calls (see #367). It'll need sorting at some point but for now it's ok. * Remove residual log * Fix notebook * Manually squish bsplines when the lines are short Makes text labels right-aligned to target instead of left-aligned to source Add kwargs overrides for line plotting * Vertically centre instead of using manual offset * Add shuffling to denoising strategies * Add transform names as edge labels * Fix test so it doesn't overlap with special case * Removes IPPM.md from docs --------- Co-authored-by: Cai Wingfield <[email protected]> Co-authored-by: Anirudh Lakra <[email protected]>
Draft
8 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
No description provided.
The text was updated successfully, but these errors were encountered: