Skip to content
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

Fix api #30

Merged
merged 14 commits into from
Dec 15, 2021
Merged

Fix api #30

merged 14 commits into from
Dec 15, 2021

Conversation

jo-mueller
Copy link
Contributor

@jo-mueller jo-mueller commented Dec 6, 2021

Description

I changed function imports in init.py on subpackage level from from subpackage import * to from . import subpackage. This forces the user to call function from a certain subpackage as biau.subpackage.function(...), the call biau.function(...) wiill no longer work.

Type of change

  • Bug-fix
  • New feature
  • Breaking change
  • Documentation update

References

Closes #28

Tests

  • I adapted existing tests, because
  • I added new tests to cover the code change
  • All tests pass with my change

Final checks

  • My change is the minimal possible work for the desired feature/fix
  • I updated the documentation where necessary to cover the change
  • I rebuilt the documentation page to document the change, if necessary

@jo-mueller
Copy link
Contributor Author

jo-mueller commented Dec 6, 2021

I fixed the imports that relied on old API calls. The tests do not cover calls in notebooks so that's something to keep in mind.

Copy link
Member

@haesleinhuepf haesleinhuepf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me @jo-mueller . Furthermore, if you write closes #28 in your PR description, the issue 28 will be automatically closed when merging this PR. Try it out ;-)

Furthermore, if the code still works without that one line I mentioned. Feel free to merge. :-)

biapol_utilities/__init__.py Show resolved Hide resolved
@jo-mueller
Copy link
Contributor Author

Looks good to me @jo-mueller . Furthermore, if you write closes #28 in your PR description, the issue 28 will be automatically closed when merging this PR. Try it out ;-)

Fancy! I changed it further up 👍

@zoccoler
Copy link
Contributor

Hi Johannes @jo-mueller,

I just tested this branch and the subpackage way of calling works (ran files in tests folder). Direct calling indeed no longer works then.

Just the last test (test_match_labels.py) yields this error (divide by zero?):

...biapol-utilities\biapol_utilities\label\_intersection_over_union.
py:50: RuntimeWarning: invalid value encountered in true_divide
  iou = overlap / (n_pixels_pred + n_pixels_true - overlap)

Also a minor thing: some places I see import biapol_utilities as biau and others it is biao

@jo-mueller
Copy link
Contributor Author

Hi Marcelo @zoccoler ,

thanks for pointing this out. I changed the inputs accordingly. The errormessages are just warnings that are usually thrown in intersection_over_union_matrix. It would be nice to to suppress this somehow - but that's for a different PR.

Feel free to merge this :)

@jo-mueller
Copy link
Contributor Author

@zoccoler @haesleinhuepf , I fixed the conflicts with the main branch so I think this could be merged as well.

@zoccoler zoccoler merged commit 6fad16b into main Dec 15, 2021
@jo-mueller jo-mueller mentioned this pull request Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check public API
3 participants