-
-
Notifications
You must be signed in to change notification settings - Fork 169
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
BLD: Change setup.py to pyproject.toml #589
BLD: Change setup.py to pyproject.toml #589
Conversation
e83d125
to
17997ab
Compare
17997ab
to
feb690d
Compare
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.
@GabrielBarberini thanks for your contribution. This is a valuable change
- It's important that we merge this PR into the
develop
branch instead of themaster
, this would preserve the consistency in our git history. Could you rebase your changes to develop please? Unless it's a critical bug, we will always send to develop first. - The
.coveragerc
(available on develop) and the.flake8
files could also be deleted as long as we transfer their rules to the new .toml file. - I believe we should add this PR to the CHANGELOG.md file, it's relevant enough.
fa1d86d
to
d143d81
Compare
|
d143d81
to
3671ff0
Compare
removes setup.py fixes pyproject typo sets max line to 88 columns Fix code style issues with Black Revert to c955e81 addresses review comments fixes optional dependencies migrates flake and unit test rules to pyproject toml addresses review comments
3671ff0
to
2ac512c
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #589 +/- ##
===========================================
- Coverage 72.99% 72.97% -0.02%
===========================================
Files 59 57 -2
Lines 9543 9596 +53
===========================================
+ Hits 6966 7003 +37
- Misses 2577 2593 +16 ☔ View full report in Codecov by Sentry. |
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 PR! Straight to the point, it solves what was requested in the issue.
We are deleting 3 files to stay with just a single 1, this is good!.
The .pylintrc file will still stand, but this is a strategical decision... Controlling pylint rules directly with the .pylintrc file must be a better idea in terms of scalability.
As I also made changes to the PR, I will ask you to wait for a second approval.
@phmbressan @MateusStano could one of you also review/approve this PR please?
Pay attention that we are deleting the setup.py file!!
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 job! Finally we have the updated Python install pyproject.toml
. The removed "hidden files" from the project structure are a nice bonus.
Pull request type
Breaking change
Additional information
Installation working after change on local machine: