-
Notifications
You must be signed in to change notification settings - Fork 40
Refactor: Update rolling-ball-plugin to new plugin standards #429
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
Conversation
* New project structure
* Versioning
* Bump version: 1.0.2 → 1.1.0-dev0
* Switch from argparse to typer
* Update logging level
* Add CI tools
* Update tests
nishaq503
left a comment
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.
There are a couple of minor changes needed here. The main change though is that the tests should be updated to only use pytest and not the unittest module.
transforms/images/rolling-ball-plugin/src/polus/transforms/images/rolling_ball/__main__.py
Outdated
Show resolved
Hide resolved
hamshkhawar
left a comment
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.
@agerardin Please have a look at my comments and let me know if you have questions
transforms/images/rolling-ball-plugin/tests/correctness_test.py
Outdated
Show resolved
Hide resolved
transforms/images/rolling-ball-plugin/src/polus/transforms/images/rolling_ball/__main__.py
Outdated
Show resolved
Hide resolved
transforms/images/rolling-ball-plugin/src/polus/transforms/images/rolling_ball/__main__.py
Outdated
Show resolved
Hide resolved
transforms/images/rolling-ball-plugin/src/polus/transforms/images/rolling_ball/__main__.py
Outdated
Show resolved
Hide resolved
transforms/images/rolling-ball-plugin/src/polus/transforms/images/rolling_ball/__main__.py
Outdated
Show resolved
Hide resolved
…base. * refactor unit tests with pytests * add new tests for the client * add exceptions for bad plugin inputs * add a preview option
* add type info to method signature * better handle plugin parameters
nishaq503
left a comment
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.
There are come more changes needed here:
- Update the email addresses of authors to the
nih.govdomain. - See my comments on the
pyproject.tomlfile. - This plugin needs a
filepatternargument as input and use it to filter input images.
| "title": "Rolling Ball", | ||
| "description": "A WIPP plugin to perform background subtraction using the rolling-ball algorithm.", | ||
| "author": "Najib Ishaq (najib.ishaq@axleinfo.com)", | ||
| "author": "Najib Ishaq (najib.ishaq@nih.gov), Antoine Gerardin (antoine.gerardin@gmail.com)", |
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.
Both email addresses should be in the nih.gov domain.
| name = "polus-plugins-transforms-images-rolling-ball" | ||
| version = "1.1.0-dev0" | ||
| description = "A WIPP plugin to perform background subtraction using the rolling-ball algorithm." | ||
| authors = ["Najib Ishaq <najib.ishaq@nih.gov>", "Antoine Gerardin <antoine,gerardin@gmail.com>"] |
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.
Email address here as well.
| packages = [{include = "polus", from = "src"}] | ||
|
|
||
| [tool.poetry.dependencies] | ||
| python = "^3.9.15" |
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.
Specify this as ">=3.9,<3.12"
|
|
||
| [tool.poetry.dependencies] | ||
| python = "^3.9.15" | ||
| bfio = "2.1.9" |
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.
Use all extras. bfio = { version = "2.1.9", extras = ["all]" }
| [[tool.poetry.source]] | ||
| name = "test" | ||
| url = "https://test.pypi.org/simple/" | ||
| default = false | ||
| secondary = true |
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.
Remove this and use the newest version of filepattern from pypi
| flake8 = "^6.0.0" | ||
| flake8-docstrings = "^1.7.0" |
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.
Remove flake8 and add ruff
|
|
||
|
|
||
| @app.command() | ||
| def main( |
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.
Add an input argument for filepattern
| "-m", | ||
| "polus.plugins.transforms.images.rolling_ball" | ||
| ], | ||
| "inputs": [ |
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.
We need a filepattern input parameter.
This PR updates the rolling ball plugin to the new plugin standard.
* New project structure
* Versioning
* Bump version: 1.0.2 → 1.1.0-dev0
* Switch from argparse to typer
* Update logging level
* Add CI tools
* Update tests