Review and modernization of the package #280
Conversation
|
Thanks @kshitij-maths can you check why all the tests are failing? We should fix them to merge the changes! |
|
@ndem0 checking. |
|
Please approve the current workflow. Let's see if it works. |
|
@ndem0 The macOS PR test is consistently failing because |
|
Ok thanks! Yes the action is not working for MacOS, they also have a warning on the main page https://github.com/s-weigand/setup-conda I would change the action so! I tried to quick search on google and there is the official action developed by Also please try to fix the Codacy issues, there are 64 new issues https://app.codacy.com/gh/mathLab/PyGeM/pull-requests/280/issues |
Okay, thanks!
Okay. |
ndem0
left a comment
ndem0
left a comment
There was a problem hiding this comment.
A huge amount of file are changed because of small formatting (double quotes instead signle one for example). Also, a lot of new file have been added, but I don't think are necessary (they should be the output of some tests).
Additionally, please try to keep few lines of changes for PR, around 100 is completely fine (here we have 6000 lines for 35 commits, definetely to large).
Please remove unnecessary file.
Please restore to the previous version all the file contined in pygem, tests and tutorials folders.
README.md
Outdated
| 1. It's generally best to start by opening a new issue describing the bug or | ||
| feature you're intending to fix. Even if you think it's relatively minor, | ||
| it's helpful to know what people are working on. Mention in the initial | ||
| issue that you are planning to work on that bug or feature so that it can | ||
| be assigned to you. | ||
| Install PyGeM in development mode with all dependencies: | ||
|
|
||
| 2. Follow the normal process of [forking][] the project, and setup a new | ||
| branch to work in. It's important that each group of changes be done in | ||
| separate branches in order to ensure that a pull request only includes the | ||
| commits related to that bug or feature. | ||
| ```bash | ||
| pip install -e ".[dev]" | ||
|
|
||
| 3. To ensure properly formatted code, please make sure to use 4 | ||
| spaces to indent the code. The easy way is to run on your bash the provided | ||
| script: ./code_formatter.sh. You should also run [pylint][] over your code. | ||
| It's not strictly necessary that your code be completely "lint-free", | ||
| but this will help you find common style issues. | ||
| ``` | ||
|
|
||
| 4. Any significant changes should almost always be accompanied by tests. The | ||
| project already has good test coverage, so look at some of the existing | ||
| tests if you're unsure how to go about it. We're using [coveralls][] that | ||
| is an invaluable tools for seeing which parts of your code aren't being | ||
| exercised by your tests. | ||
| This installs the package in editable mode with all testing, documentation, and | ||
| quality tools. | ||
|
|
||
| 5. Do your best to have [well-formed commit messages][] for each change. | ||
| This provides consistency throughout the project, and ensures that commit | ||
| messages are able to be formatted properly by various git tools. | ||
| ### Submitting a patch | ||
|
|
||
| 6. Finally, push the commits to your fork and submit a [pull request][]. Please, | ||
| remember to rebase properly in order to maintain a clean, linear git history. | ||
| 1. It's generally best to start by opening a new issue describing the bug or | ||
| feature you're intending to fix. Even if you think it's relatively minor, | ||
| it's helpful to know what people are working on. Mention in the initial | ||
| issue that you are planning to work on that bug or feature so that it can | ||
| be assigned to you. | ||
|
|
||
| 2. Follow the normal process of [forking][] the project, and setup a new | ||
| branch to work in. It's important that each group of changes be done in | ||
| separate branches in order to ensure that a pull request only includes the | ||
| commits related to that bug or feature. | ||
|
|
||
| 3. To ensure properly formatted code, please make sure to use 4 | ||
| spaces to indent the code. The easy way is to run on your bash the provided | ||
| script: ./code_formatter.sh. You should also run [pylint][] over your code. | ||
| It's not strictly necessary that your code be completely "lint-free", | ||
| but this will help you find common style issues. | ||
|
|
||
| 4. Any significant changes should almost always be accompanied by tests. The | ||
| project already has good test coverage, so look at some of the existing | ||
| tests if you're unsure how to go about it. We're using [coveralls][] that | ||
| is an invaluable tools for seeing which parts of your code aren't being | ||
| exercised by your tests. | ||
|
|
||
| 5. Do your best to have [well-formed commit messages][] for each change. | ||
| This provides consistency throughout the project, and ensures that commit | ||
| messages are able to be formatted properly by various git tools. | ||
|
|
||
| 6. Finally, push the commits to your fork and submit a [pull request][]. | ||
| Please, | ||
| remember to rebase properly in order to maintain a clean, linear git | ||
| history. |
There was a problem hiding this comment.
I don't think we need to revert the complete README to its original version. We've simply reorganized it for better clarity. If you look at the rendered file rather than the raw code, you'll see that the content is essentially the same, perhaps more polished and structured. The overwhelming number of added and deleted lines you see here is likely because I rewrote the file completely, rather than making changes line by line.
pyproject.toml
Outdated
| "future", | ||
| "numpy>=1.21.0", | ||
| "scipy>=1.7.0", | ||
| "matplotlib>=3.5.0" |
kshitij-maths
force-pushed
the
master
branch
from
a6e6f44 to
3c11eb1
Compare
2 participants
This PR introduces several modernization improvements to the package, including: