Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RSDK-9631] Add deprecation warnings to DiscoverComponents #818

Merged
merged 2 commits into from
Jan 16, 2025

Conversation

randhid
Copy link
Member

@randhid randhid commented Jan 16, 2025

This PR adds warnings to client calls and Deprecation docstrings to the discover_components API in the robot client. To my knowledge, this SDK does not have a server side implementation of this API.

I also pre-emptively removed the tests that call this api - let me know if you'd rather keep those.

I don't usually contribute to the python sdk - so here are my local uv run make test warnings and failures - if they matter:

============================================================== FAILURES ===============================================================
____________________________________________________ test_ndarrays_to_flat_tensors ____________________________________________________

    @pytest.mark.filterwarnings("ignore::DeprecationWarning")
    def test_ndarrays_to_flat_tensors():
        output = ndarrays_to_flat_tensors(MockMLModel.INTS_NDARRAYS)
        assert len(output.tensors) == 4
        assert all(name in output.tensors.keys() for name in ["0", "1", "2", "3"])
        assert type(output.tensors["0"].int8_tensor.data) is builtins.bytes
        bytes_buffer = output.tensors["0"].int8_tensor.data
        assert np.array_equal(np.frombuffer(bytes_buffer, dtype=np.int8).reshape(output.tensors["0"].shape), MockMLModel.INT8_NDARRAY)
>       assert np.array_equal(np.array(output.tensors["1"].int16_tensor.data, dtype=np.int16), MockMLModel.INT16_NDARRAY)
E       OverflowError: Python integer 4294967295 out of bounds for int16

tests/test_mlmodel_utils.py:56: OverflowError
========================================================== warnings summary ===========================================================
tests/test_board.py::TestClient::test_stream_ticks
  /opt/homebrew/Cellar/[email protected]/3.13.1/Frameworks/Python.framework/Versions/3.13/lib/python3.13/asyncio/base_events.py:745: RuntimeWarning: coroutine method 'aclose' of 'BoardClient.stream_ticks.<locals>.read' was never awaited
    self._ready.clear()
  Enable tracemalloc to get traceback where the object was allocated.
  See https://docs.pytest.org/en/stable/how-to/capture-warnings.html#resource-warnings for more info.

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
======================================================= short test summary info =======================================================
FAILED tests/test_mlmodel_utils.py::test_ndarrays_to_flat_tensors - OverflowError: Python integer 4294967295 out of bounds for int16
============================================== 1 failed, 645 passed, 1 warning in 12.27s ==============================================
make: *** [test] Error 1

@randhid randhid requested a review from a team as a code owner January 16, 2025 00:24
@randhid randhid requested review from stuqdog and lia-viam January 16, 2025 00:24
Comment on lines 764 to 768
warnings.warn(
"RobotClient.discover_components is deprecated. It will be removed on March 10 2025. Use the DiscoverySerrvice APIS instead.",
DeprecationWarning, stacklevel=2)
LOGGER.warning(
"RobotClient.discover_components is deprecated. It will be removed on March 10 2025. Use the DiscoverySerrvice APIS instead.")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[q] For reviewers - which one is better?

@randhid randhid merged commit 203eba8 into main Jan 16, 2025
12 checks passed
@randhid randhid deleted the deprecate-discover-components branch January 16, 2025 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants