imi-bigpicture / wsidicom

Python package for reading DICOM WSI file sets.
Apache License 2.0
33 stars 5 forks source link

Add option to provide a session to the web client #117

Closed psavery closed 11 months ago

psavery commented 11 months ago

We would like to create the requests.Session object on our own and pass it into WsiDicomWebClient, since we are not using an object derived from AuthBase to create it.

This PR adds support to pass a session to WsiDicomWebClient directly.

erikogabrielsson commented 11 months ago

Hi @psavery and thanks for the PR.

This is a reasonable change. Would you also like to add a Changed entry to CHANGELOG.md describing the change?

psavery commented 11 months ago

Hi @psavery and thanks for the PR.

This is a reasonable change. Would you also like to add a Changed entry to CHANGELOG.md describing the change?

Done!

psavery commented 11 months ago

FYI, there are other optional arguments on the DICOMwebClient __init__ method. See here.

In the future, if we keep needing to add arguments, we might want to consider just having the caller create the DICOMwebClient on their own and pass it in as an argument.

erikogabrielsson commented 11 months ago

FYI, there are other optional arguments on the DICOMwebClient __init__ method. See here.

In the future, if we keep needing to add arguments, we might want to consider just having the caller create the DICOMwebClient on their own and pass it in as an argument.

I agree, it would be good if WsiDicomWebClient init took a DICOMwebClient client. If you want, you can implement this and move the current init to a create-classmethod.

psavery commented 11 months ago

I agree, it would be good if WsiDicomWebClient init took a DICOMwebClient client. If you want, you can implement this and move the current init to a create-classmethod.

Done! Let me know what you think

erikogabrielsson commented 11 months ago

Looks good. Two things i forgot:

psavery commented 11 months ago

Done. Let me know what you think.

erikogabrielsson commented 11 months ago

Great work. Thanks! I will merge this as soon as the checks runs OK.

psavery commented 11 months ago

@erikogabrielsson Let me know when you are planning to have another release of wsidicom so we can access these features on PyPI.

erikogabrielsson commented 10 months ago

@psavery Release 0.13..0 includes these changes. Thanks again for the contribution.

psavery commented 10 months ago

Thanks @erikogabrielsson!