-
Notifications
You must be signed in to change notification settings - Fork 0
Isaac 2025 Work Package #15
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
base: main
Are you sure you want to change the base?
Conversation
…ter script into 2d_plate/
… pip install with pyproject.toml
…d building multi-classifier
lukethehuman
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.
Great first pass. The code is very neat, nicely laid out, and readable, though there are a number of style improvements I've suggested which would be a big improvement. Largely these are the same suggestions you'll get automatically by using linting tools and/or pycodestyle and pydocstyle.
Taking the example file as as example of code a user might write to use Icarus, the current API requires a lot more user code than I think is necessary. It would be good to aim for an example that runs the MWE in minimal lines of code (without sacrificing customisability of the tool), alongside an example demonstrating the full customisability.
The code installed and ran successfully with a fresh build of proteus and the latest pypi version of pyvale; I've recommended simplifying the installation by version tagging pyvale in the pyproject.toml requirements.
Co-authored-by: Luke Humphrey <37630574+lukethehuman@users.noreply.github.com>
Co-authored-by: Luke Humphrey <37630574+lukethehuman@users.noreply.github.com>
Co-authored-by: Luke Humphrey <37630574+lukethehuman@users.noreply.github.com>
No description provided.