Suggestions for improvement

[jbussemaker]

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

[kyleniemeyer]

@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]

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]

@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:

• https://dev.to/2320sharon/making-sense-of-pyprojecttoml-setupcfg-and-setuppy-2o6m
• https://setuptools.pypa.io/en/latest/userguide/quickstart.html
  • In particular: https://setuptools.pypa.io/en/latest/userguide/declarative_config.html

[kyleniemeyer]

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

[spinjet]

Progress so far:

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, @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]

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]

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]

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]

Great, thanks for the update @spinjet!

[spinjet]

As mentioned in openjournals/joss-reviews#6110 (comment)
Automated testing with CI is now fully implemented.

[jbussemaker]

@spinjet great progress here

One comment about the .gitignore file: please also remove all content from the repository that would be "ignored". For example, build and dist should not be present.

[spinjet]

Added a almost completed section for State of the Field with relevant references.

[spinjet]

Dear @jbussemaker.
I've completed the paper improvements.
Added a State of the Field section providing more context on the code and comparison with other frameworks/approaches. Introduced more references, including to the methods used in PDOPT. These also address the points @e-dub made in #4.

Hope this is satisfactory from the paper side, I'll focus on publishing in PyPI and having an online version of the documentation (especially for the API).

[spinjet]

@jbussemaker

Online documentation is complete and online.
https://pdopt-code.readthedocs.io/en/latest/

Last task is creating a release version and publishing to PyPI.

[spinjet]

Code has been published on PyPI: https://pypi.org/project/pdopt/
And I've made an official release as version 0.5.0.

@jbussemaker Let me know if you are satisfied with the completion of the tasks.

[jbussemaker]

@spinjet after these small updates, I am satisfied:

• Link to the documentation website in the repository (click on the settings cog next to "About" and add the URL)
• Fix small typos in the main readme
  • PyPI is styled exactly like that: "PyPI"
  • "Alternatevly it is possible to donwload the release..."
• Also update installation instructions on the documentation website
• On the documentation main page there is a typo in the acknowledgements: "cotnributed"

[spinjet]

@jbussemaker Thank you again for the feedback.
I've made the corrections you asked for.

[jbussemaker]

Thank you! Then I will go ahead and close this issue, and recommend publication :)

Very nice work!