pymeasure / pyleco

Python implementation of the Laboratory Experiment COntrol (LECO) protocol
MIT License
9 stars 3 forks source link

Remove json dependency #63

Closed BenediktBurger closed 6 months ago

BenediktBurger commented 6 months ago

Closes #5

codecov[bot] commented 6 months ago

Codecov Report

Attention: Patch coverage is 93.10345% with 18 lines in your changes are missing coverage. Please review.

Project coverage is 87.63%. Comparing base (4cc7cca) to head (549f628). Report is 2 commits behind head on main.

Files Patch % Lines
pyleco/json_utils/rpc_server_definition.py 84.81% 7 Missing and 5 partials :warning:
pyleco/json_utils/errors.py 88.88% 4 Missing :warning:
pyleco/coordinators/coordinator.py 84.61% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #63 +/- ## ========================================== + Coverage 87.28% 87.63% +0.35% ========================================== Files 33 36 +3 Lines 2626 2846 +220 Branches 314 343 +29 ========================================== + Hits 2292 2494 +202 - Misses 280 293 +13 - Partials 54 59 +5 ``` | [Flag](https://app.codecov.io/gh/pymeasure/pyleco/pull/63/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pymeasure) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/pymeasure/pyleco/pull/63/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pymeasure) | `87.63% <93.10%> (+0.35%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pymeasure#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

BenediktBurger commented 6 months ago

Working for a week in the lab.

bilderbuchi commented 6 months ago
  • Removes dependency on jsonrpc2-objects and jsonrpc2-pyclient. These two dependencies can be written in two small files, which enables us to support older versions of Python than the dependencies allow.

    • Adds a small version of RPCServer for use in Python<3.9 instead of the version of openrpc package. This self written version does not yet support fully openrpc creation of method description, therefore it is good to keep openrpc as a dependency for those python versions, where it is available.

Closes #5

A few thoughts from skimming the PR:

BenediktBurger commented 6 months ago

Thanks for your comments

* I would caution against supporting py37, as it has been EOL since last year. Py38 follows this year.

The idea was, to include pyleco into pymodaq, which supports python 3.8 and @seb5g mentioned, that he would like to use it on an old system, which only allows python 3.7. However, I noticed, that python 3.7 is not achievable in #61 , so I dropped that idea.

* The package metadata has `requires-python = ">=3.9"`, so installation should not be possible on lower Python versions anyway. I guess you need py38, nevertheless?

This PR is the first one of two, which adds objects. The second one (#64 ) with more changes, changes also that requirement.

* It's good that you restricted the self-written code to just use it when necessary, as it's easier to remove later, and reduce maintenance burden.

Especially as it is not as complete as the imported code.