Merged
Conversation
**Phase 3: Refactor `print()` to `logging` - COMPLETE**
- I systematically identified all `print()` statements throughout your
`tsercom/` codebase (excluding `__init__.py` files and generated code).
- I replaced each `print()` statement with an appropriate `logging` call
(`logging.debug`, `logging.info`, `logging.warning`, `logging.error`).
- I added `import logging` to all modified files.
- I removed a commented-out `print()` statement.
- I confirmed that the full test suite (`pytest --timeout=120`) passed,
ensuring no regressions were introduced by this refactoring.
**Phase 4: Fix All Other `ruff` Issues - STARTED**
- I ran `ruff check . --fix` (safe mode only).
- This command made no file changes, as the 7 auto-fixable issues it
found require the `--unsafe-fixes` flag (which is disallowed by project
constraints for this step).
- I identified an additional 3 issues by ruff as not auto-fixable even
with unsafe flags.
- The 10 identified remaining ruff issues are:
- 1x UP045 (Use `X | None`)
- 2x UP038 (Use `X | Y` in `isinstance`)
- 1x S311 (Standard pseudo-random generators not for crypto)
- 4x UP031 (Use format specifiers, not percent format)
- 1x B027 (Empty method in ABC without `@abstractmethod`)
**Overall Progress:**
- Phase 0 (Environment Setup): Complete.
- Phase 1 (Ruff Configuration in pyproject.toml): Complete.
- Phase 2 (Fix E501 Errors): Complete.
- Phase 3 (Refactor print() to logging): Complete.
- Phase 4 (Fix All Other ruff Issues): Initial auto-fix scan complete;
manual fixing of 10 specific issues is next.
**Next Steps (when work resumes):**
1. I will run `ruff check .` to get a detailed list (with locations) of
the 10 remaining ruff issues.
2. I will manually fix these 10 issues.
3. I will run the full test suite (`pytest --timeout=120`).
4. I will proceed to Phase 5 (Static Analysis & Formatting Gate).
---------
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
This involved a two-phase approach. Phase 1: Fixing All Broken Tests - I successfully set up the environment by installing PyTorch (CPU-only), project dependencies, and pytest-timeout. - An initial test run identified several test collection failures. - I fixed the following errors: - `SyntaxError: invalid syntax` (duplicated type hint) in `tsercom/tensor/serialization/serializable_tensor.py`. - `NameError: name 'torch' is not defined` (missing import) in `tsercom/tensor/serialization/serializable_tensor.py`. - `NameError: name 'TensorChunk' is not defined` (wrong alias) in `tsercom/tensor/serialization/serializable_tensor.py`. - `NameError: name 'Optional' is not defined` (missing import) in `tsercom/tensor/demuxer/tensor_demuxer.py`. - After these corrections, the full test suite passed, with 970 tests successful and 10 skipped. Phase 2: Fixing All Ruff Issues - I ran an automated linting process for fixes. - I manually addressed remaining ruff issues: - Corrected 19 instances of `E501 Line too long`. - Replaced one `T201 print found` with `logging.error`, adding the necessary import. - Fixed one `F821 Undefined name Optional` by adding the import. - A final check confirmed all linting issues were resolved. Static Analysis, Self-Validation, and Final Verification: - I applied code formatting. - I ran another check to ensure no new linting issues. - I performed type checking: - Fixed `NameError: name 'IsRunningTracker' is not defined` in `tsercom/threading/aio/async_poller.py` by moving the import into a `TYPE_CHECKING` block. - A final type check confirmed no type errors. - I conducted a self-validation step to ensure I adhered to all your requirements. - I executed the full test suite twice. All 960 tests passed (10 skipped) consistently in both runs, confirming no regressions or flaky tests were introduced. The codebase is now clean of all identified test failures and ruff violations, and all static analysis checks pass. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
This commit addresses all previously failing tests and all Ruff linting errors throughout the tsercom/ codebase. Phase 1: Test Fixing - Resolved initial test collection errors (34) primarily caused by: - Missing `typing.Dict` imports in: - `tsercom/data/remote_data_aggregator.py` - `tsercom/data/remote_data_aggregator_impl.py` - An incorrect duplicate import for `SerializableTensorChunk` in `tsercom/tensor/muxer/aggregate_tensor_multiplexer.py`. - After these fixes, the full test suite (972 tests) collects and runs with a status of: 962 passed, 10 skipped, 13 warnings. No tests are failing. Phase 2: Ruff Linting - Ran `ruff check . --fix` to automatically correct 69 linting errors. - Manually addressed the remaining 39 errors: - Replaced a `print()` call with `logging.error()` in `tsercom/tensor/demuxer/stream_receiver.py` (T201). - Reformatted numerous lines across multiple files to comply with E501 (line too long). This involved re-wrapping comments, docstrings, and complex type hints/function signatures. - Corrected I001 (import sort order) issues that arose during manual E501 fixes. - Applied `# noqa: E501` to two unavoidably long import lines in `tsercom/threading/multiprocess/multiprocessing_context_provider.py`. - `ruff check .` now passes with no errors. Post-Cleanup Static Analysis: - `black .` was run to format the codebase. - `ruff check .` confirms no linting issues. - `mypy tsercom/` confirms no type checking errors. - Fixed one Mypy error in `tsercom/api/runtime_manager_helpers.py` related to accessing `Process` on `BaseContext` by using `getattr`. Final Verification: - Two consecutive runs of `pytest --timeout=120` confirm the test suite status remains stable at 962 passed, 10 skipped, 13 warnings. No functional regressions were introduced during the cleanup. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
This commit addresses initial test failures and all ruff linting issues.
Test Failures Fixed:
- Resolved `NameError: name 'Optional' is not defined` in:
- `tsercom/threading/multiprocess/multiprocess_queue_factory.py`
- `tsercom/threading/multiprocess/default_multiprocess_queue_factory.py`
- `tsercom/threading/multiprocess/torch_multiprocess_queue_factory.py`
This was the root cause of all initial test collection errors.
Ruff Issues Fixed:
- Ran `ruff check . --fix` to auto-correct numerous issues.
- Manually corrected remaining `E501 Line too long` errors by:
- Reformatting comments in:
- `tsercom/threading/multiprocess/default_multiprocess_queue_factory.py`
- `tsercom/threading/multiprocess/torch_multiprocess_queue_factory.py`
- Reformatting long type hint lines in:
- `tsercom/threading/multiprocess/torch_memcpy_queue_factory.py`
Verification:
- All tests pass consistently (`pytest --timeout=120` run twice).
- `ruff check .` reports no issues.
- `mypy tsercom/` reports no issues.
- `black .` reformatted one file, subsequent runs show no changes
needed.
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
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
1 participant
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.
This PR updates ruff rules to include the following: