Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates the project tooling from pixi/setuptools to an uv + hatch-based workflow, refreshes dependency/tooling configuration, and includes a set of refactors and bug fixes across clustering/graph/stream utilities plus documentation updates.
Changes:
- Move packaging/versioning and CI workflows to
uv(dependency groups,uv sync,uv build) and add docs checks with strict mkdocs builds. - Refactor cluster internals (IDKC/PSKC/KCluster) and add an optional
force_assign_unassignedbehavior with corresponding tests. - Update docs site navigation/content and enable notebook execution during docs builds.
Reviewed changes
Copilot reviewed 56 out of 57 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject_old.toml | Adds a snapshot of prior setuptools/pixi config. |
| pyproject.toml | Switches to hatch/uv-dynamic-versioning + uv dependency groups; updates deps and tooling config. |
| mkdocs.yml | Adds FAQ to nav; enables mkdocs-jupyter execution. |
| ikpykit/trajectory/utils.py | Modernizes typing imports/annotations. |
| ikpykit/trajectory/tests/test_ikat.py | Tightens expected exception type for not-fitted behavior. |
| ikpykit/trajectory/dataloader/base.py | Removes ABC base from Dataset. |
| ikpykit/timeseries/tests/test_iktod.py | Makes exception-message matching deterministic via escaped regex. |
| ikpykit/timeseries/anomaly/_iktod.py | Adds stacklevel to warnings. |
| ikpykit/stream/tests/test_icid.py | Adds per-module pytest timeout. |
| ikpykit/stream/cluster/utils/serialize_trees.py | Cleans imports and uses str.format for output writing. |
| ikpykit/stream/cluster/utils/logger.py | Adds typing for class var; simplifies class declaration. |
| ikpykit/stream/cluster/utils/file_utils.py | Improves readability of parsing + avoids redundant open args. |
| ikpykit/stream/cluster/utils/dendrogram_purity_pool.py | Small modernization (super call, prints, variable naming). |
| ikpykit/stream/cluster/utils/dendrogram_purity.py | Uses public tqdm import and cleans variable naming. |
| ikpykit/stream/cluster/utils/deltasep_utils.py | Cleans module header/docs and minor style tweaks. |
| ikpykit/stream/cluster/utils/Graphviz.py | Refactors string formatting + suppresses exceptions more cleanly. |
| ikpykit/stream/cluster/_streakhc.py | Modernizes typing (PEP 604 unions) and removes unused imports. |
| ikpykit/stream/cluster/_inode.py | Modernizes typing and minor comprehension cleanups. |
| ikpykit/stream/changedetect/_icid.py | Fixes mutable default arg + doc/formatting tweaks. |
| ikpykit/kernel/_isokernel.py | Improves error/warn messages and adds stacklevel. |
| ikpykit/kernel/_ik_inne.py | Removes unused variable binding. |
| ikpykit/group/utils.py | Modernizes typing imports/annotations. |
| ikpykit/graph/utils.py | Simplifies transpose handling. |
| ikpykit/graph/tests/test_isographkernel.py | Uses LIL then CSR to avoid sparse efficiency warnings. |
| ikpykit/graph/tests/test_ikgod.py | Uses LIL then CSR in synthetic adjacency construction; minor test edits. |
| ikpykit/graph/_isographkernel.py | Docstring dash normalization; adds stacklevel; minor loop var cleanup. |
| ikpykit/graph/_ikgod.py | Adds stacklevel to warnings. |
| ikpykit/graph/init.py | Reorders __all__. |
| ikpykit/cluster/tests/test_pskc.py | Removes unused test variable. |
| ikpykit/cluster/tests/test_idkc.py | Removes unused test vars; adds tests for force_assign_unassigned. |
| ikpykit/cluster/_pskc.py | Refactors clustering steps into helper methods; reuses KCluster helpers. |
| ikpykit/cluster/_kcluster.py | Refactors delete logic and adds shared helper methods. |
| ikpykit/cluster/_idkc.py | Adds force_assign_unassigned option + refactors post-process logic. |
| ikpykit/cluster/init.py | Reorders __all__. |
| ikpykit/anomaly/tests/test_inne.py | Removes unused variables in timing test. |
| ikpykit/anomaly/_inne.py | Improves messages/warnings formatting and adds stacklevel. |
| ikpykit/anomaly/_idkd.py | Improves messages/warnings formatting and adds stacklevel. |
| ikpykit/init.py | Reorders __all__. |
| examples/plot_inne.py | Import ordering/style cleanup. |
| docs/user_guides/table-of-contents.md | Updates INNE link to notebook. |
| docs/user_guides/inne.ipynb | Updates example to import/use INNE and refreshes notebook content. |
| docs/releases/releases.md | Adds a new changelog section for this release. |
| docs/quick-start/how-to-install.md | Recommends uv pip install; updates unstable install command. |
| docs/faq/table-of-contents.md | Replaces unrelated FAQ content with IKPyKit-specific FAQ. |
| docs/examples/examples_english.md | Updates INNE link to notebook. |
| docs/README.md | Updates badges and docs links; recommends uv. |
| README.md | Updates badges; recommends uv installation. |
| .python-version | Sets local dev Python to 3.13. |
| .pre-commit-config.yaml | Replaces hooks set; adds ruff format, validate-pyproject, codespell, etc. |
| .github/workflows/pytest.yml | Switches unit tests to uv sync + pytest matrix including 3.13. |
| .github/workflows/pypi.yml | Modernizes build/publish using uv build and OIDC publish; adds GitHub release step. |
| .github/workflows/lint.yml | Switches lint to uv sync + black/ruff. |
| .github/workflows/docs.yml | Adds strict docs build job with notebook execution. |
| .github/workflows/deploy-gh-pages.yml | Adds docs pre-check and versioned deployment via mike. |
| .github/workflows/ci.yml | Adds top-level CI workflow calling lint/tests/docs. |
| .github/dependabot.yml | Updates dependabot configuration and labels for dependency updates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -164,7 +171,8 @@ def fit(self, X, y=None): | |||
| if self.n_init_samples > X.shape[0]: | |||
| self.n_init_samples = X.shape[0] | |||
| raise warn( | |||
There was a problem hiding this comment.
raise warn(...) is not valid exception handling: warn() returns None, so this will raise a TypeError at runtime. If the intent is to emit a warning and continue, call warn(...) without raise. If the intent is to stop execution, raise an actual exception type (e.g., ValueError) and do not call warn here.
| raise warn( | |
| warn( |
| np.random.rand(5, 2) | ||
|
|
There was a problem hiding this comment.
This is a no-op expression (np.random.rand(5, 2)) whose result is unused. With ruff’s bugbear rules enabled, this can be flagged as a “useless expression” and fail linting; remove it or assign it to a variable that’s used in the test.
| self.points_.remove(points) | ||
| self.reduce_kernel_mean_(X) | ||
| elif isinstance(points, Iterable): | ||
| missing_points = [p for p in points if p not in self.points_] | ||
| if missing_points: | ||
| raise ValueError(f"Points {missing_points} not in cluster {self.id}") | ||
| for p in points: | ||
| self.points_.remove(p) | ||
| self.reduce_kernel_mean_(X) |
There was a problem hiding this comment.
delete_points updates points_ before calling reduce_kernel_mean_, but reduce_kernel_mean_ uses self.n_points to scale the existing mean. After removal, self.n_points no longer matches the mean’s previous sample count, so the updated kernel_mean_ will be incorrect. Call reduce_kernel_mean_ before mutating points_, or pass the previous point count into the mean update logic.
| self.points_.remove(points) | |
| self.reduce_kernel_mean_(X) | |
| elif isinstance(points, Iterable): | |
| missing_points = [p for p in points if p not in self.points_] | |
| if missing_points: | |
| raise ValueError(f"Points {missing_points} not in cluster {self.id}") | |
| for p in points: | |
| self.points_.remove(p) | |
| self.reduce_kernel_mean_(X) | |
| # Update kernel mean before mutating points_ so that self.n_points | |
| # still reflects the original sample count used in kernel_mean_. | |
| self.reduce_kernel_mean_(X) | |
| self.points_.remove(points) | |
| elif isinstance(points, Iterable): | |
| missing_points = [p for p in points if p not in self.points_] | |
| if missing_points: | |
| raise ValueError(f"Points {missing_points} not in cluster {self.id}") | |
| # As above, update kernel mean before removing any points so that | |
| # self.n_points matches the count used for the current kernel_mean_. | |
| self.reduce_kernel_mean_(X) | |
| for p in points: | |
| self.points_.remove(p) |
update to uv manage and fix some bugs