-
Notifications
You must be signed in to change notification settings - Fork 3
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
Version 0.0.1 #13
Version 0.0.1 #13
Changes from 50 commits
50d4674
b5fe070
2b102ea
b75bdeb
99854f8
b4d84f6
f896453
485d6ba
4d4f7fa
fab0aba
cc4b422
2cf73bf
36d31fb
d2afab0
88f05f2
bbbd1d6
ab0972e
fdc0e9b
bed0724
a78d557
c07e06e
ff2e3d2
ddcdc4b
0333bdf
2b03027
6adc09c
7e8fea2
e7db3e1
712e85d
b12239c
04a7dd3
1602e54
677f866
4663129
3d4760a
9d89009
4afb8cd
e49dda6
db24b5d
b7752b8
7b24392
2081bfe
97338b9
fd16826
faac9f5
3d70110
abc0577
411b0e3
206a673
29ac0f8
e1ace81
e5e2825
d21022c
7d0d7aa
3151a18
e5d01e8
868f572
5e5adea
1940698
2b6cbd3
e30613d
5b2bab0
cf911c7
adf3fd5
c01d89d
586020a
1af657c
4c07c16
c147041
2453207
77bbe64
62a26c6
c824be8
3e5d80e
1735dd6
fe0c6b1
3d51dae
7ac3fa3
f0349d5
dad8dd2
4b5a480
49c6943
d9da532
9fbadc3
08c698b
ff83b6b
35a5dad
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,7 +28,7 @@ jobs: | |
- { python: "3.10", os: "macos-latest", session: "tests" } | ||
- { python: "3.10", os: "ubuntu-latest", session: "typeguard" } | ||
- { python: "3.10", os: "ubuntu-latest", session: "xdoctest" } | ||
- { python: "3.10", os: "ubuntu-latest", session: "docs-build" } | ||
- { python: "3.6", os: "ubuntu-latest", session: "docs-build" } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure I understand the need to use nox together with github actions. in my opinion this makes the whole thing more complicated. I don't see what this brings. for instance such a matrix would do exactly the same matrix:
python: [3.6, 3.7, 3.8, 3.9, 3.10]
os: [ubuntu-latest, macos-latest, windows-latest] and you would create 3 jobs which would be more explicit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree. Moreover, i thought this library was mainly going to be used in osparc services? why ensuring support in macos and windows? |
||
|
||
env: | ||
NOXSESSION: ${{ matrix.session }} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,3 +8,4 @@ | |
/docs/_build/ | ||
/src/*.egg-info/ | ||
__pycache__/ | ||
*ignore* |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
# Heps setup basic env development | ||
GitHK marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
# poetry is required on your system | ||
# suggested installation method | ||
# or refer to official docs | ||
# https://python-poetry.org/docs/ | ||
.PHONY: install-poetry | ||
install-poetry: | ||
curl -sSL https://raw.githubusercontent.com/python-poetry/poetry/master/get-poetry.py | python - | ||
|
||
# install deve dependecis as sugested by coockicutter | ||
GitHK marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# https://cookiecutter-hypermodern-python.readthedocs.io/en/2021.11.26/quickstart.html | ||
.PHONY: install-dev | ||
install-dev: | ||
pip install nox nox-poetry | ||
|
||
.PHONY: tests | ||
tests: # run tests on lowest python interpreter | ||
nox -r -s tests -p 3.6 | ||
|
||
.PHONY: nox-36 | ||
nox-36: # runs nox with python 3.6 | ||
nox -p 3.6 | ||
|
||
.PHONY: tests-dev | ||
tests-dev: | ||
pytest -vv -s --exitfirst --failed-first --pdb tests/ | ||
|
||
.PHONY: docs | ||
docs: # runs and displays docs | ||
#runs with py3.6 change the noxfile.py to use different interpreter version | ||
nox -r -s docs | ||
|
||
|
||
.PHONY: codestyle | ||
codestyle: # runs codestyle enforcement | ||
isort . | ||
black . | ||
|
||
.PHONY: mypy | ||
mypy: # runs mypy | ||
nox -p 3.6 -r -s mypy |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,7 @@ coverage: | |
status: | ||
project: | ||
default: | ||
target: "100" | ||
target: "99" | ||
patch: | ||
default: | ||
target: "100" | ||
target: "99" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,20 @@ | ||
Usage | ||
===== | ||
|
||
.. click:: osparc_control.__main__:main | ||
:prog: osparc-control | ||
:nested: full | ||
Please have a look at the examples folder. | ||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Restructured text is definitively a very rich format for doc that we have used it extensively in s4l manuals. There it was necessary because of the complexity of the doc (>200+ pages !) and due to the lack of maturity of other solutions at the time . Nonetheless, I am not sure it is really justified here. So far all python repos have been using two engines (docsify and mkdocs) both based in markdown doc (which is different from restructured). Adding restructured to the equation requires to learn yet another tool and language ...and I am not convinced it is really justified here. Can you perhaps point out what is the advantage wrt markdown here? I guess also @KZzizzle should have a say here as well provided most of the doc has been created by her. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would imagine that the user documentation will pull from but won't necessarily be the same as this. It's up to you guys what type of documentation is most idea here. |
||
Minimual examples | ||
================= | ||
|
||
In one terminal run ``python examples/basic/requester.py``: | ||
|
||
.. literalinclude:: ../examples/basic/requester.py | ||
:language: python | ||
|
||
|
||
|
||
In a second terminal run ``python examples/basic/replyer.py``: | ||
|
||
.. literalinclude:: ../examples/basic/replyer.py | ||
:language: python |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
# About | ||
|
||
This example consists of a `time_solver`. Which can add, the current time by a provided value. | ||
|
||
# Usage | ||
|
||
In one terminal run `sidecar_controller.py`. | ||
In a second terminal run `time_solver.py`. It will load data from the `sidecar_solver.py` to use when communicating with the `sidecar_controller.py` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
from osparc_control import ControlInterface | ||
|
||
|
||
control_interface = ControlInterface( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could we not call this module a sidecar? this is not following the sidecar pattern in any way is it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for me, the anming inside the examples is not super important - I understand enough about what is happening but also have no problem with the rename. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe instead of control_interface you could call it controlled_solver ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've renamed this to |
||
remote_host="localhost", exposed_interface=[], remote_port=1234, listen_port=1235 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could we have some sensible defaults (for instance for the ports or so?) do we really need to set them each time? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. actually I see the ports are inversed. so what about having a RemoteControl or Controller implementation and a Controlled that already set up the ports automatically? Also the remote_host is most of the time localhost, set it as default? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As said below In theory this |
||
) | ||
control_interface.start_background_sync() | ||
GitHK marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
# add_internal_time | ||
|
||
add_params = {"a": 10} | ||
print("Will add ", add_params) | ||
request_id = control_interface.request_with_delayed_reply( | ||
"add_internal_time", params=add_params | ||
) | ||
|
||
has_result = False | ||
result = None | ||
while not has_result: | ||
has_result, result = control_interface.check_for_reply(request_id=request_id) | ||
|
||
print("result of addition", result) | ||
|
||
# get_time | ||
|
||
print("getting solver time") | ||
solver_time = control_interface.request_with_immediate_reply("get_time", timeout=1.0) | ||
random_int = control_interface.request_with_immediate_reply( | ||
"random_in_range", timeout=1.0, params={"a": 1, "b": 3} | ||
GitHK marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) | ||
print("solver time", solver_time) | ||
|
||
print("sending command to print internal status") | ||
control_interface.request_without_reply("print_status") | ||
|
||
|
||
control_interface.stop_background_sync() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
from osparc_control import CommandManifest | ||
from osparc_control import CommandParameter | ||
from osparc_control import CommnadType | ||
from osparc_control import ControlInterface | ||
|
||
|
||
GitHK marked this conversation as resolved.
Show resolved
Hide resolved
|
||
command_add = CommandManifest( | ||
action="add_internal_time", | ||
description="adds internal time to a provided paramter", | ||
params=[ | ||
CommandParameter(name="a", description="param to add to internal time"), | ||
], | ||
command_type=CommnadType.WITH_DELAYED_REPLY, | ||
) | ||
|
||
command_get_time = CommandManifest( | ||
action="get_time", | ||
description="gets the time", | ||
params=[], | ||
command_type=CommnadType.WITH_IMMEDIATE_REPLY, | ||
) | ||
|
||
command_print_solver_status = CommandManifest( | ||
action="print_status", | ||
description="prints the status of the solver", | ||
params=[], | ||
command_type=CommnadType.WITHOUT_REPLY, | ||
GitHK marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) | ||
|
||
|
||
control_interface = ControlInterface( | ||
GitHK marked this conversation as resolved.
Show resolved
Hide resolved
|
||
remote_host="localhost", | ||
exposed_interface=[command_add, command_get_time, command_print_solver_status], | ||
remote_port=1235, | ||
listen_port=1234, | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
from time import sleep | ||
|
||
from sidecar_solver import command_add | ||
from sidecar_solver import command_get_time | ||
from sidecar_solver import command_print_solver_status | ||
from sidecar_solver import control_interface | ||
|
||
from osparc_control.core import ControlInterface | ||
from osparc_control.models import CommandRequest | ||
|
||
|
||
def handle_inputs(time_solver: "TimeSolver", request: CommandRequest) -> None: | ||
if request.action == command_add.action: | ||
sum_result = time_solver._add(**request.params) | ||
time_solver.control_interface.reply_to_command( | ||
request_id=request.request_id, payload=sum_result | ||
) | ||
return | ||
|
||
if request.action == command_get_time.action: | ||
time_solver.control_interface.reply_to_command( | ||
request_id=request.request_id, payload=time_solver.time | ||
) | ||
return | ||
|
||
if request.action == command_print_solver_status.action: | ||
print("Solver internal status", time_solver) | ||
# finally exit | ||
time_solver.can_continue = False | ||
GitHK marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return | ||
|
||
|
||
class TimeSolver: | ||
def __init__( | ||
self, initial_time: float, control_interface: ControlInterface | ||
) -> None: | ||
self.time = initial_time | ||
self.control_interface = control_interface | ||
|
||
# internal time tick | ||
self.sleep_internal: float = 0.1 | ||
self.can_continue: bool = True | ||
|
||
def __repr__(self) -> str: | ||
return ( | ||
f"<{self.__class__.__name__} time={self.time}, " | ||
f"sleep_interval={self.sleep_internal}>" | ||
) | ||
|
||
def _add(self, a: float) -> float: | ||
return self.time + a | ||
|
||
def run(self) -> None: | ||
"""main loop of the solver""" | ||
while self.can_continue: | ||
# process incoming requests from remote | ||
for request in self.control_interface.get_incoming_requests(): | ||
handle_inputs(time_solver=self, request=request) | ||
|
||
# process internal stuff | ||
self.time += 1 | ||
sleep(self.sleep_internal) | ||
|
||
|
||
def main() -> None: | ||
control_interface.start_background_sync() | ||
GitHK marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
solver = TimeSolver(initial_time=0, control_interface=control_interface) | ||
solver.run() | ||
|
||
control_interface.stop_background_sync() | ||
|
||
|
||
if __name__ == "__main__": | ||
main() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
import random | ||
import time | ||
|
||
from osparc_control import CommandManifest | ||
from osparc_control import CommandParameter | ||
from osparc_control import CommnadType | ||
from osparc_control import ControlInterface | ||
|
||
random_in_range_manifest = CommandManifest( | ||
action="random_in_range", | ||
description="gets the time", | ||
params=[ | ||
CommandParameter(name="a", description="lower bound for random numbers"), | ||
CommandParameter(name="b", description="upper bound for random numbers"), | ||
], | ||
command_type=CommnadType.WITH_IMMEDIATE_REPLY, | ||
) | ||
|
||
control_interface = ControlInterface( | ||
remote_host="localhost", | ||
exposed_interface=[random_in_range_manifest], | ||
remote_port=2346, | ||
listen_port=2345, | ||
) | ||
control_interface.start_background_sync() | ||
|
||
wait_for_requests = True | ||
while wait_for_requests: | ||
for command in control_interface.get_incoming_requests(): | ||
if command.action == random_in_range_manifest.action: | ||
random_int = random.randint( # noqa: S311 | ||
command.params["a"], command.params["b"] | ||
) | ||
control_interface.reply_to_command( | ||
request_id=command.request_id, payload=random_int | ||
) | ||
wait_for_requests = False | ||
|
||
# allow for message to be delivered | ||
time.sleep(0.01) | ||
|
||
control_interface.stop_background_sync() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
from osparc_control import ControlInterface | ||
|
||
control_interface = ControlInterface( | ||
remote_host="localhost", exposed_interface=[], remote_port=2345, listen_port=2346 | ||
) | ||
|
||
control_interface.start_background_sync() | ||
|
||
random_int = control_interface.request_with_immediate_reply( | ||
"random_in_range", timeout=10.0, params={"a": 1, "b": 10} | ||
) | ||
print(random_int) | ||
assert 1 <= random_int <= 10 # noqa: S101 | ||
|
||
control_interface.stop_background_sync() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why did you choose flake? How does it compare to what we already know/have ?
My understanding is that this is covered by
black
andpylint
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because it already came in bundled with flake8, it does the same job.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these entries are amazingly clear 🤣