Refactor code structure and update CI workflows - #96
Merged
Conversation
…d other bells and whistles Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
- Update contigs_to_positions to use list conversion for groupby chunks. - Rename truncate_reads and filter_bamfile to _truncate_reads and _filter_bamfile for consistency. - Remove main.py as CLI entry point and replace with rich-click integration in tests. - Enhance view.py to improve fasta record sorting logic. - Revamp test_commands.py to utilize Click's CliRunner for better CLI testing. - Update test_distance_law.py to reflect changes in function names and improve test structure. - Remove unused docopt dependency from project. Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
There was a problem hiding this comment.
Pull request overview
Refactors the project into a modern src/-based Python package with a new rich-click CLI entrypoint, and updates packaging/CI/Docker to use pyproject.toml + uv + Ruff.
Changes:
- Migrate packaging from
setup.py/requirements.txttopyproject.toml(src/layout) and add Ruff/pre-commit tooling. - Replace the legacy docopt-based CLI with a
rich-click-based CLI and update CLI tests accordingly. - Revamp CI workflows (lint/test/publish) and Docker build/install flow to match the new packaging approach.
Reviewed changes
Copilot reviewed 42 out of 44 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_pipeline.py | Updates pipeline tests and aligner parametrization. |
| tests/test_io.py | Test cleanup/formatting changes; fixes temp file closing calls. |
| tests/test_hicstuff.py | Import ordering/formatting changes and minor test adjustments. |
| tests/test_filter.py | Formatting/cleanup of filter tests. |
| tests/test_doctests.py | Minor import formatting. |
| tests/test_distance_law.py | Updates tests to new internal distance-law helper names and formatting. |
| tests/test_digest.py | Formatting and minor cleanup of digest tests. |
| tests/test_commands.py | Rewrites CLI tests to use click.testing.CliRunner against the new hicstuff.cli:cli. |
| src/hicstuff/view.py | Formatting/cleanup and fixes FASTA reordering logic to sort records properly. |
| src/hicstuff/stats.py | New stats module for parsing pipeline logs and emitting text/JSON summaries. |
| src/hicstuff/pipeline.py | Refactors imports/formatting and updates logging/IO patterns. |
| src/hicstuff/log.py | Minor import reordering/cleanup. |
| src/hicstuff/iteralign.py | Refactors/renames internal helpers and simplifies subprocess/logging code. |
| src/hicstuff/io.py | Refactors imports, improves error handling, and adjusts defaults for mutable args. |
| src/hicstuff/hicstuff.py | Refactors formatting and adjusts several utility implementations. |
| src/hicstuff/filter.py | Refactors formatting and narrows exception handling around plotting. |
| src/hicstuff/distance_law.py | Refactors formatting, makes several helpers “private”, and fixes NaN handling. |
| src/hicstuff/digest.py | Refactors formatting and improves error reporting on fragment ID/position inconsistencies. |
| src/hicstuff/cutsite.py | Refactors formatting and simplifies string building. |
| src/hicstuff/cli.py | New rich-click CLI implementation replacing legacy docopt entrypoint. |
| src/hicstuff/init.py | Adds runtime version discovery via importlib.metadata and updates author metadata. |
| setup.py | Removed legacy setuptools entrypoint/build configuration. |
| setup.cfg | Removed legacy wheel build config. |
| requirements.txt | Removed legacy dependency list in favor of pyproject.toml. |
| README.md | Updates docs to new CLI help output, install workflow (uv), and CI badge/workflows. |
| pyproject.toml | Adds PEP 621 project metadata, dependencies, scripts, Ruff/pytest/coverage configuration. |
| MANIFEST.in | Removed legacy manifest configuration. |
| Makefile | Removed legacy build/test helper targets. |
| hicstuff/version.py | Removed generated version file (version now resolved from package metadata). |
| hicstuff/stats.py | Removed legacy stats module (replaced by src/hicstuff/stats.py). |
| hicstuff/main.py | Removed docopt-based CLI entrypoint. |
| environment.yml | Removed conda env file (CI/Docker now install deps differently). |
| Dockerfile | Updates installation to use uv and installs system tools via micromamba. |
| doc/notebooks/demo_api.ipynb | Formatting and minor code snippet updates. |
| doc/conf.py | Removes legacy encoding header. |
| .pylintrc | Removed legacy pylint configuration (migrating to Ruff). |
| .pre-commit-config.yaml | Adds pre-commit hooks for Ruff + basic hygiene checks. |
| .github/workflows/pypi-publish.yml | Removed legacy Twine-based publish workflow. |
| .github/workflows/publish.yml | New publish workflow for PyPI (uv) + GHCR Docker builds. |
| .github/workflows/ci.yml | New CI workflow using Ruff + pytest across a Python version matrix. |
| .github/workflows/build.yml | Removed legacy micromamba-based CI workflow. |
| .appveyor.yml | Removed legacy Windows CI config. |
Comments suppressed due to low confidence (6)
src/hicstuff/pipeline.py:246
- The mapped-read percentage calculation is off by a factor of 100 due to multiplying by 100 twice. This will log incorrect mapping rates (e.g., 5000% instead of 50%). Compute the percent once (e.g., 100 * mapped / total) and format it directly.
src/hicstuff/io.py:151 save_sparse_matrix()usesdtype is intto pick the integer format, buts_mat.dtypeis typically a NumPy dtype (e.g.,dtype('int64')), so this condition will almost always be false and integer matrices will be saved with float formatting. Use a NumPy-safe check (e.g.,np.issubdtype(dtype, np.integer)) to select the integer format.
src/hicstuff/hicstuff.py:1829corrcoef_sparse()currently ignores theBparameter: thevstack((A, B))result is not assigned, so the correlation is always computed onAonly. AlsoA.copy()is called without using the copy. Restore the previous behavior by assigning the stacked matrix to the variable used for downstream computations.
src/hicstuff/hicstuff.py:2046split_matrix()callslen(chunk), butchunkfromitertools.groupby()is an iterator, solen(chunk)raisesTypeErrorat runtime. Convert the chunk to a list first (or consume it while counting) and use its length for slicing.
src/hicstuff/hicstuff.py:1410max_intra_distance = max(len(contigs == u) ...)is incorrect:contigs == uproduces a boolean array andlen(...)returns the total length ofcontigsfor everyu, somax_intra_distancebecomesnregardless of contig sizes. This breaks the intra/inter distance split logic; use a count of elements per contig (e.g.,np.sum(contigs == u)).
src/hicstuff/distance_law.py:917plot_ps_slope()builds a colormap iterator and advances it withnext(cols), but the resulting color is never applied to the plotted lines. Either pass the extracted color intologlog/semilogx(so the rainbow mapping is used) or remove the unused iterator to avoid misleading dead code.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <copilot@github.com>
…q count nonetheless Co-authored-by: Copilot <copilot@github.com>
…uff module to _misc module
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.
Refactor the code structure by organizing files into
src/hicstuffand updating thepyproject.toml. Use rich-click for CLI. Remove branch restrictions in the CI workflow and adjust aligner parameters in tests. Enhance linting and fix pytest errors to improve code quality. Update Dockerfile dependencies and CI workflows for better integration with GitHub Container Registry.