SciCatProject / pyscicat

A python client library for interacting with the SciCat data catalog backend.
Other
0 stars 9 forks source link

Remove code duplication in client #27

Closed jl-wynen closed 2 years ago

jl-wynen commented 2 years ago

There currently is a lot of duplicate code in the client. And a lot of methods are mostly copies of each other. And other are implemented slightly differently even though they do equivalent work. This makes it hard to see similarities.

This PR unifies all interaction with the backend. This is mainly a refactor, but there are some behavioural changes:

This is not just about cleaning up code. It will also help us test downstream code by creating a fake client. This is easier with the refactor because _call_endpoint and _send_to_scicat operate on structured data and not preformatted URLs and dictionaries. I will explain this in a separate issue.

Unfortunately, test coverage is quite sparse, so I am not positive that all refactorings are correct. Please review carefully!

Question: datasets_datablock_create uses 'origdatablocks'. Should this be 'datablocks' instead?

dylanmcreynolds commented 2 years ago

Thanks for this PR. This is a lot of changes, but it appears pretty clean.

The change to the urllib causes a couple of issues. flake8 is unhappy:

./pyscicat/client.py:734:9: F821 undefined name 'urllib'
./pyscicat/client.py:750:9: F821 undefined name 'urllib'

If these same problems also cause pytest to fail.

Can you please look at this and get both flake8 and pytest running?

jl-wynen commented 2 years ago

Done.