Refactored RemoteDataOrganizer with SortedList and added an interpolation#172
Open
Refactored RemoteDataOrganizer with SortedList and added an interpolation#172
Conversation
Here's what I accomplished in our previous sessions:
- Environment Setup: I installed PyTorch, project dependencies, and additional tools.
- Dependency Updates: I added 'sortedcontainers' to pyproject.toml and README.md.
- Refactoring RemoteDataOrganizer:
- I switched the internal data storage to sortedcontainers.SortedList (timestamp-keyed).
- I updated methods for SortedList compatibility.
- I implemented get_interpolated_at() for linear data interpolation.
In this session:
I focused on getting back to our plan, specifically resuming at Phase 3 (Testing) after an interruption. I didn't make any new code changes to the main application or test files this time.
My next step is to begin updating the existing unit tests in 'tsercom/data/remote_data_organizer_unittest.py' to align with the SortedList refactoring. After that, I'll add new tests for the interpolation method.
…olation, and updating the initial tests.
Here's what I've done so far:
- I've set up the environment, updated `pyproject.toml` and `README.md`, refactored `RemoteDataOrganizer` to use `SortedList`, and implemented the `get_interpolated_at()` method.
- For test preparations, I removed a junk file, updated `test_initialization` for `SortedList`, and added new tests for `get_interpolated_at`.
- Static analysis shows that the main code in `remote_data_organizer.py` is clean. The test code in `remote_data_organizer_unittest.py` has had significant type hinting added and some logical corrections made for `SortedList` behavior.
Currently, I'm facing some issues:
- The first test run showed 18 failures and 1 error in `remote_data_organizer_unittest.py`.
- The main reason for this is that the test setup code wasn't fully updated from `deque` methods (like `appendleft`/`append`) to `SortedList.add()`.
- This led to incorrect list ordering in assertions and some new test failures.
- I also noticed a missing fixture in one of the new tests.
- I also observed 7 unrelated failures in `tsercom/tensor_e2etest.py`.
Here's what I plan to do next:
- I'll focus on fixing the failing unit tests.
- My immediate action will be to correct `remote_data_organizer_unittest.py` by:
- Replacing all `deque` data manipulation methods in the test setups with `SortedList.add()`.
- Correcting the assertion list orders.
- Fixing the fixture error and other specific test failures.
- Then, I'll run the tests again, aiming for all relevant tests to pass.
Here's what I've done so far:
- I refactored the core of RemoteDataOrganizer to use SortedList.
- I added the get_interpolated_at method.
- I updated pyproject.toml and README.md.
- I added initial new tests for get_interpolated_at.
- Static analysis on remote_data_organizer.py (main code) is clean.
- I successfully fixed syntax errors in remote_data_organizer_unittest.py that were present from a very early faulty script. The test file is now syntactically valid.
My current goal is to fix the failing unit test logic in remote_data_organizer_unittest.py.
Here's what went wrong in my latest attempt:
- My effort to correct critical test logic in remote_data_organizer_unittest.py did not succeed.
- The Python script (correct_tests_v2.py) I wrote for this had a NameError because of mis-scoped helper functions. It didn't make its intended changes.
- I reverted the test file to its state before this failed attempt.
Here's the current state of remote_data_organizer_unittest.py:
- It is syntactically valid.
- However, it STILL contains logical errors causing test failures:
- DummyExposedDataForOrganizerTests lacks a .replace() method (or suitable __init__) for get_interpolated_at to reconstruct instances. This is the likely cause of new interpolation tests failing (returning None).
- Many assertions of list content (e.g., `assert list(organizer._RemoteDataOrganizer__data) == [...]`) likely still expect newest-first (deque) order, not SortedList's oldest-first (ascending timestamp) order.
- Mypy compliance for the test file is still low.
My immediate next step MUST be to correctly implement the changes that correct_tests_v2.py was supposed to make, particularly to DummyExposedDataForOrganizerTests and list order assertions. This requires fixing the Python script and trying again.
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.
I've refactored RemoteDataOrganizer with SortedList and added an interpolation method.
Here's what I've done so far: