-
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
Add unit test and refactor TiledDataset #23
Conversation
f5b914f
to
33fab49
Compare
Thanks for the hard work! I did several rounds of the test on my local end, and would suggest a few more changes with the introduction of the dependency injection, most of them are related to the additional layer of container for the actual masks, (referring to the "one_up_container" we discussed back on Tuesday). And for the unit tests, I ran into one error and it seems to be related to the import of |
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.
I've added a few comments with code suggestions to accommodate the dependency injection change.
Also I do have one more suggestion to request for the change of allocate_array_space
function in utils.py
, but I was not able to find a way to point it out in this review as no prior changes of that file have been made in this PR. Please let me know what's a good way to point it out.
I think this is ready for you to test again. I refactored the test methods. |
Thanks for the modification. Now pytest works fine on my end, and all checks passed. In order to execute Unless @taxe10 and @Wiebke have additional thoughts, I believe this PR is ready to be merged. |
This PR adds a unit test that creates a database and runs it in github actions. The unit test starts up a tiled server and connects to it.
It also slightly refactors the
TiledDataset
, using dependency injection to send tiled clients as init parameters rather than having it construct its own internally. This makes this type of unit testing easier. Without that, then we would have to monkey patch theTiledDataset
and itsdata_client
andmask_client
members. That's possible, but dependency injection is more flexible and readable.