search

pymeasure / pyleco

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

Reorganize json objects #65

Closed BenediktBurger closed 7 months ago

BenediktBurger commented 7 months ago

Now that we have our own json-objects and errors file, we can put all errors (even the LECO ones and the pyleco INVALID_SERVER_RESPONSE) into one file instead of keeping errors distributed over several files (errors, json_utils.errors, rpc_generator).

codecov[bot] commented 7 months ago

Codecov Report

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

Project coverage is 87.68%. Comparing base (db1e795) to head (5b3b193).

Files Patch % Lines
pyleco/json_utils/errors.py 71.42% 4 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #65 +/- ## ========================================== + Coverage 87.58% 87.68% +0.10% ========================================== Files 36 36 Lines 2859 2867 +8 Branches 346 347 +1 ========================================== + Hits 2504 2514 +10 + Misses 295 293 -2 Partials 60 60 ``` | [Flag](https://app.codecov.io/gh/pymeasure/pyleco/pull/65/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/65/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pymeasure) | `87.68% <88.57%> (+0.10%)` | :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 7 months ago

What do you think about the reorganization, that all errors are in one file and all json rpc objects in another file, @bilderbuchi ?

bilderbuchi commented 7 months ago

What do you think about the reorganization, that all errors are in one file and all json rpc objects in another file, @bilderbuchi ?

Sounds good in general to keep error definitions together. From a quick glance, it seems strange that all (even generic) errors are in the json_utils module, I would have thought to prefer the pyleco.errors module as the more fitting place to collect all the errors (json-related and other)?

BenediktBurger commented 7 months ago

Thanks for your comment. Basically all errors are json errors, either defikned by jsonrpc or defined by leco for use in jsonrpc messages. The LECO error codes (for example for NOT_SIGNED_IN) are according to jsonrpc specifications and a re meant to be sent via jsonrpc messages, so they are json errors.

If you deem it better, I'll move the errors file to the root directory.

bilderbuchi commented 7 months ago

If you deem it better, I'll move the errors file to the root directory.

I don't have strong feelings either way.

BenediktBurger commented 7 months ago

Thanks for your comments. I looked at the code again, and the errors are all json related, therefore, I'll keep them in the json directory.