Suggestions for improvement

jbussemaker commented 9 months ago

I'm currently reviewing the JOSS submission, and I have some suggestions to improve code quality and usability. I did not install/run any code yet, I'll get to that soon.

Software Engineering (Python)

Important:

[x] Add a .gitignore file to remove all contents from the repository that should not be there, for example pycache folder, dist/build folders. You can go to https://www.toptal.com/developers/gitignore/ and enter Python, PyCharm, Windows, etc. to get a good default .gitignore file.
[x] Add tests, absolutely essential to ensure your software functions as intended, and keeps doing so as you add/change functionality over time. I recommend using pytest
- [x] Consider automatically running tests using Github actions CI/CD
[x] Use one way to declare dependencies: better to integrate this in the setup.py file (even better to use setup.cfg)
[x] Add a pyproject.toml file: this file tells Python dependency management tools what tool should be used to install the library (in PDOPT's case: setuptools). Some info: https://godatadriven.com/blog/a-practical-guide-to-setuptools-and-pyproject-toml/
[x] Add community guidelines for: contributing, reporting bugs, getting support
[x] Improve code styling, e.g. by adhereing to PEP8 or using a formatter like black
[x] Remove unused code (cpu_test.py; a big chunk of data.py)

Suggestions:

[x] Consider managing stable versions by creating tags and releases
[x] Consider publishing your code on PyPI
[x] In your current documentation file, Figure 1.2 seems to be missing some refs
[x] Consider hosting the documentation as a website, for example on readthedocs.io
- [x] To create the documentation, consider using modern tools like mkdocs and Jupyter Notebooks instead of a PDF file (I know LaTeX is quite common in research circles, but for software it's usually much nicer to navigate a website with search, and Jupyter Notebooks are great for explaining code as it allows mixing code, execution output and explanatory text in one interactive document)
- [x] You can use mkdocstrings to automatically generate your API reference automatically (this also makes it easier to keep in sync with your actual code)

Paper

[x] Add a comparison to other approaches/software ("state of the field")
- [x] All the paper references are own references of previous publications, I would also have expected some references to theory (e.g. the probabilistic model or set-based design)

@kyleniemeyer I'd like to know your opinion about the paper authors: I can only find code contributions by Andrea Spinelli, however Timoleon Kipouros is also listed as an author

kyleniemeyer commented 9 months ago

@jbussemaker regarding the author list: JOSS does not require that all paper authors have contributed source code, since people can contribute in other ways (direction, etc.)

spinjet commented 8 months ago

I'm currently reviewing the JOSS submission, and I have some suggestions to improve code quality and usability. I did not install/run any code yet, I'll get to that soon.

Software Engineering (Python)

Important:

[ ] Add a .gitignore file to remove all contents from the repository that should not be there, for example pycache folder, dist/build folders. You can go to https://www.toptal.com/developers/gitignore/ and enter Python, PyCharm, Windows, etc. to get a good default .gitignore file.

[ ] Add tests, absolutely essential to ensure your software functions as intended, and keeps doing so as you add/change functionality over time. I recommend using pytest

[ ] Consider automatically running tests using Github actions CI/CD

[ ] Use one way to declare dependencies: better to integrate this in the setup.py file (even better to use setup.cfg)

[ ] Add a pyproject.toml file: this file tells Python dependency management tools what tool should be used to install the library (in PDOPT's case: setuptools). Some info: https://godatadriven.com/blog/a-practical-guide-to-setuptools-and-pyproject-toml/

[ ] Add community guidelines for: contributing, reporting bugs, getting support

[ ] Improve code styling, e.g. by adhereing to PEP8 or using a formatter like black

[ ] Remove unused code (cpu_test.py; a big chunk of data.py)

Suggestions:

[ ] Consider managing stable versions by creating tags and releases

[ ] Consider publishing your code on PyPI

[ ] In your current documentation file, Figure 1.2 seems to be missing some refs

[ ] Consider hosting the documentation as a website, for example on readthedocs.io

[ ] To create the documentation, consider using modern tools like mkdocs and Jupyter Notebooks instead of a PDF file (I know LaTeX is quite common in research circles, but for software it's usually much nicer to navigate a website with search, and Jupyter Notebooks are great for explaining code as it allows mixing code, execution output and explanatory text in one interactive document)

[ ] You can use mkdocstrings to automatically generate your API reference automatically (this also makes it easier to keep in sync with your actual code)

Paper

[ ] Add a comparison to other approaches/software ("state of the field")

[ ] All the paper references are own references of previous publications, I would also have expected some references to theory (e.g. the probabilistic model or set-based design)

@kyleniemeyer I'd like to know your opinion about the paper authors: I can only find code contributions by Andrea Spinelli, however Timoleon Kipouros is also listed as an author

Dear @jbussemaker, I've started working on these and I've already introduced the .gitignore file.

Could you please elaborate on the dependencies...? I am a bit inexperienced on creating python packages, this is my first time. I was using an external text file to load the dependencies, so it was easier while I was developing the software. Should I have them hardcoded in the setup.py file, or external? Do you perhaps have a tutorial or guideline link I could look into?

jbussemaker commented 8 months ago

@spinjet yes sure! The link I provided for the point after the "dependencies" actually contains a bit about specifying dependencies in a setup.cfg file (https://godatadriven.com/blog/a-practical-guide-to-setuptools-and-pyproject-toml/).

Some other links I found through a quick google query:

kyleniemeyer commented 8 months ago

Here is another good guide to making an installable package: https://packaging.python.org/en/latest/tutorials/packaging-projects/

spinjet commented 8 months ago

Progress so far:

Software Engineering (Python)

Important:

[x] Add a .gitignore file to remove all contents from the repository that should not be there, for example pycache folder, dist/build folders. You can go to https://www.toptal.com/developers/gitignore/ and enter Python, PyCharm, Windows, etc. to get a good default .gitignore file.
[ ] Add tests, absolutely essential to ensure your software functions as intended, and keeps doing so as you add/change functionality over time. I recommend using pytest
- [ ] Consider automatically running tests using Github actions CI/CD
[x] Use one way to declare dependencies: better to integrate this in the setup.py file (even better to use setup.cfg)
[x] Add a pyproject.toml file: this file tells Python dependency management tools what tool should be used to install the library (in PDOPT's case: setuptools). Some info: https://godatadriven.com/blog/a-practical-guide-to-setuptools-and-pyproject-toml/
[x] Add community guidelines for: contributing, reporting bugs, getting support
[x] Improve code styling, e.g. by adhereing to PEP8 or using a formatter like black
[x] Remove unused code (cpu_test.py; a big chunk of data.py)

Suggestions:

[ ] Consider managing stable versions by creating tags and releases
[ ] Consider publishing your code on PyPI
[ ] In your current documentation file, Figure 1.2 seems to be missing some refs
[ ] Consider hosting the documentation as a website, for example on readthedocs.io
- [ ] To create the documentation, consider using modern tools like mkdocs and Jupyter Notebooks instead of a PDF file (I know LaTeX is quite common in research circles, but for software it's usually much nicer to navigate a website with search, and Jupyter Notebooks are great for explaining code as it allows mixing code, execution output and explanatory text in one interactive document)
- [ ] You can use mkdocstrings to automatically generate your API reference automatically (this also makes it easier to keep in sync with your actual code)

Paper

[x] Add a comparison to other approaches/software ("state of the field")
- [x] All the paper references are own references of previous publications, I would also have expected some references to theory (e.g. the probabilistic model or set-based design)

@kyleniemeyer, @jbussemaker Thank you so far for your help, I've adopted the new .toml approach with a setuptools .cfg file. I'll try to have it uploaded on PyPI for easy install through pip, albeit I may not be able to maintain the code 24/7.

I'm going to be addressing the tests soon, I'll try to add a few using pytest. I'll also address the paper and extend it with some background referencing directly some of the building blocks that aren't directly developed by me (Set-based design being one). I'm not sure for other approaches, I reckon I could mention some design space exploration approaches or general use of optimisation as a discovery tool.

kyleniemeyer commented 8 months ago

Hi @spinjet - good progress! I might suggest that for simplicity, you just include your package's dependencies directly in the pyproject.toml file, rather than a separate file: https://packaging.python.org/en/latest/guides/writing-pyproject-toml/#dependencies-optional-dependencies

jbussemaker commented 8 months ago

And I just wanted to note that publishing on PyPI is also possible by CI/CD workflow, to automate most of it:

https://packaging.python.org/en/latest/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows/

spinjet commented 8 months ago

Dear @kyleniemeyer @jbussemaker Thank you again for your help.

I've been writing tests and integrating them with pytest and GitHub actions, I am close to completion to this task. I need to fix the testing of the optimisation module as it seems there's some randomness I cannot control (the module relies on stochastic optimisation approaches).

Hopefully this should be done by the week along with the CI for publishing! You should be able to see the progress in the repository commits.

kyleniemeyer commented 8 months ago

Great, thanks for the update @spinjet!