bugfix/missing-plugin-metadata #59
Conversation
BrianWhitneyAI
requested review from
toloudis,
hughes036,
evamaxfield,
SeanLeRoy,
yrkim98 and
tyler-foster
|
Looking for some input on best way to test. Looks like there is some issues with python 3.12... |
bioio/tests/test_bio_image.py
Outdated
|
|
||
| try: | ||
| # Install the plugin | ||
| subprocess.check_call([sys.executable, "-m", "pip", "install", package_name]) |
There was a problem hiding this comment.
I think this test would be safer if you just made a Mock Reader that didn't have an author in it. I'm honestly not sure how easy that would be to do (to act like it was installed), but it might be handy for other unit tests in this repo as well.
There was a problem hiding this comment.
added dummy plugin in 2c49cd8
bioio/plugins.py
Outdated
| if "author" in dist.metadata: | ||
| print(f" Author : {dist.metadata['author']}") | ||
| else: | ||
| print(" Author : Not specified") |
There was a problem hiding this comment.
I suggest trying to do it so that the string " Author : " is only in one print statement... something more or less like
author = dist.metadata.get("author")
print(f"Author:{author if author is not None else "Not Specified"}")
This feels incrementally less error prone.
There was a problem hiding this comment.
resolved in 2c49cd8
BrianWhitneyAI
force-pushed
the
bugfix/missing-plugin-metadata
branch
from
7a25d42
to
2c49cd8
Compare
| @@ -0,0 +1 @@ | |||
| recursive-include bioio *.toml | |||
There was a problem hiding this comment.
If anyone knows how to do this in pyproject.toml that is probably better.
There was a problem hiding this comment.
is this trying to include the dummy plugin? I think for the manifest we want to exclude test files right? or am i misunderstanding
There was a problem hiding this comment.
also there's a check-manifest section in the main pyproject.toml
resolved! |
| @@ -0,0 +1 @@ | |||
| recursive-include bioio *.toml | |||
There was a problem hiding this comment.
is this trying to include the dummy plugin? I think for the manifest we want to exclude test files right? or am i misunderstanding
|
|
||
| try: | ||
| # Install the plugin | ||
| subprocess.check_call([sys.executable, "-m", "pip", "install", DUMMY_PLUGIN]) |
There was a problem hiding this comment.
I love the mock plugin, just wish there were a simpler way to "install" it. IDK this is fine tho.
There was a problem hiding this comment.
this is the simplest I have seen. I will do some looking...
Description of Changes
The purpose of this PR is to resolve #57. It seems that a plugins distribution metadata does not guarantee that there is an author or license attached to a plugin, thus a key error was raised. This PR adds a conditional for each as well as a test to run dump plugins to help us be aware of future issues with plugin reporting.