Closed roomrys closed 5 months ago
[!WARNING]
Rate limit exceeded
@roomrys has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 32 minutes and 13 seconds before requesting another review.
How to resolve this issue?
After the wait time has elapsed, a review can be triggered using the `@coderabbitai review` command as a PR comment. Alternatively, push new commits to this PR. We recommend that you space out your commits to avoid hitting the rate limit.How do rate limits work?
CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our [FAQ](https://coderabbit.ai/docs/faq) for further information.Commits
Files that changed from the base of the PR and between 0053cda7f8ef1dcf50004554b50a70b8850716e7 and a5a47c52eaf3224b884294df796133032f21c045.
The recent changes focus on enhancing the initialization and handling of ZeroMQ (ZMQ) ports within the SLEAP GUI. This involves refactoring the logic to set default port values, find free ports, and handle errors when ports are unavailable. These updates ensure smoother communication setup and improve the robustness of the SLEAP GUI's port configuration.
File | Change Summary |
---|---|
sleap/gui/learning/runners.py | Refactored zmq_ports initialization using inference_params to ensure default port values if not explicitly provided. |
sleap/gui/widgets/monitor.py | Introduced logic to set up ZMQ ports for communication if not provided, including setting default ports, finding free ports, and adding error handling for cases where ports cannot be found after attempts. |
tests/gui/test_monitor.py | Added assertions and tests to ensure correct ZMQ port settings in LossViewer instances and tested behavior related to ZMQ ports and batch showing. |
sequenceDiagram
participant User
participant MonitorWidget
participant ZMQ
User ->> MonitorWidget: Initialize
MonitorWidget ->> MonitorWidget: Check provided ZMQ ports
alt Ports provided
MonitorWidget ->> ZMQ: Bind to provided ports
else No ports provided
MonitorWidget ->> MonitorWidget: Set default ports
MonitorWidget ->> ZMQ: Find free ports
alt Free ports found
MonitorWidget ->> ZMQ: Bind to free ports
else No free ports found
MonitorWidget ->> User: Raise error
end
end
In the land of SLEAP, where ports do bind,
A rabbit hops with code refined.
Default ports set, free ones sought,
Errors handled, no battles fought.
Communication flows, smooth and clear,
With ZMQ, there's naught to fear.
🐇✨
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
Attention: Patch coverage is 63.15789%
with 7 lines
in your changes missing coverage. Please review.
Project coverage is 74.09%. Comparing base (
7ed1229
) to head (a5a47c5
). Report is 13 commits behind head on develop.
Files | Patch % | Lines |
---|---|---|
sleap/gui/widgets/monitor.py | 75.00% | 4 Missing :warning: |
sleap/gui/learning/runners.py | 0.00% | 3 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Description
After the changes added in:
1780
when running inference, the zmq ports are not on the inference GUI and so were not being passed into the
LossViewer
. Because of this, when trying to run inference, we would get this error:This PR does not add the ZMQ ports to the inference GUI, but instead makes changes so that even if ZMQ ports were not found in the GUI params, default values will be used.
Types of changes
Does this address any currently open issues?
[list open issues here]
Outside contributors checklist
Thank you for contributing to SLEAP!
:heart:
Summary by CodeRabbit
New Features
MonitorWidget
, including default port handling and dynamic free port allocation.Tests
LossViewer
instances.