-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
refactor: Refactor pyrorisks #83
Conversation
pytest-cov = "^5.0.0" | ||
|
||
|
||
[tool.poetry.group.app.dependencies] |
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.
[QUESTION] should we name it app or api?
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 can work. I named it app
to match the top-level directory and the fast API app/API convention, but api
is straightforward!
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.
[QUESTION] Why use both ruff and black ? Ruff can be a direct replacement for black, so it seems it is redundant
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.
True, thanks for catching that!
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.
Thank you for this PR !! I have a little question, I'm wondering if you chose Python 3.10 for technical reasons/compatibility? The 3.11 (or even 3.12) would give many QOLs for typing, interpreter errors, etc.
Fair question; the main concern was to avoid incompatibilities and reduce the surface for build errors. These few points are enough to rule out The package is compatible with Python 3.10 and 3.11, but I went with the 3.10 version for the docker image and the CI. The idea is to introduce relevant multi-platform and multi-version builds later! |
The PR introduces the following changes:
refactor:
chore:
style: Fix type hints, formatting, and linting
docs: Remove deprecated modules from docs
To speed up the API deployment, the refactored version of the repository only targets Linux distributions and python 3.10; multi-platform and multi-version builds will come later. It will also allow us to assess if a mix of poetry and conda is required for installation across platforms (cf
pyrovision
,pyroengine
).I plan on adding unit test workflow with the next PR!
Any feedback is welcomed 🌲