r-costa / sbml2hyb Goto Github PK

This package contains python code that is unpackaged, this means that it is problematic for others to get and install it on their own systems. There are several ways to package code these days, usually using an appropriate setup.cfg or pyproject.toml file such that it can be cloned and subsequently installed with pip

Typically, code lives in a folder called src/ within which you should make a folder with the package name sbml2hyb, where you put your code. This is important for packaging code correctly

More information on the merits of the src/ layout at https://hynek.me/articles/testing-packaging/. This will help towards #1

This code currently does not contain any documentation. There are a few ways to go about doing that to make it useful for potential users:

1. Add a "getting started" example to the README
2. Use sphinx to auto-generate documentation based on docstrings and weave them together with tutorials

See https://www.youtube.com/watch?v=azf6yzuJt54 for motivation on better documentation - there should be several parts to documentation:

1. Technical documentation (i.e., all functions are self-explanatory what they do based on their docstrings)
2. High-level documentation (i.e., tutorials for the thing 99% of users will want to do with your package after reading your paper)

As a follow-up to #1 and #2, it should be abundantly clear from the README how to install the code

After packing the code in #1, it would be paramount to deploy the code to the Python Package Index (PyPI) such that users can effortlessly install with pip

The code in this repository doesn't appear to have consistent styling. This can be easily addressed with black, which reformats code in a deterministic way. It's not perfect in how it looks, but at least it makes it more standard for others to read.

Further, you should use tools like flake8 and pylint to find other common mistakes/pitfalls in the way the code is written. Keep in mind that your code is effectively the methods section of your manuscript, and that it should be meant to be read by all users.

This package is missing important metadata in its setup.py. You can use the following commands to iteratively add the things that are missing:

git clone https://github.com/r-costa/sbml2hyb
cd sbml2hyb
pip install pyroma
pyroma --min=10 .

As a follow-up to #4, the documentation should be deployed to ReadTheDocs. This has the advantage that users don't have to build the documentation themselves, and that it automatically stays up-to-date with changes in the code.

Zenodo offers to make up a persistent backup of the repository. This is important in case GitHub goes down, if you decided to delete the repository, or any other situation. You can connect your github account to zenodo then make a release on this repo to create a DOI. Then you can add the DOI to the README and your paper.

Dear @r-costa,

I have been invited to review your application note entitled "SBML2HYB: a Python interface for SBML compatible hybrid modelling" submitted to Oxford Bioinformatics. As an application note is an advertizement for some interesting code, it's paramount that the code is good and useful. Therefore, I've provided a first round of code review that points out some standard practices for packaging, documentation, testing, code quality assurance, and automation that should be completed before the next round of review.

Greetings from Bonn, Germany. Have a nice day.
-Charlie Hoyt

Here's a table of contents of the issues I've raised:

• #8
• #1
• #2
• #3
• #4
• #5
• #6
• #7
• #9
• #10
• #11

There aren't any tests for the code in this repository. Among other things, this doesn't inspire confidence in a potential user that the code really does what it says it should. Further, without tests, it's hard to be confident that improvements don't break other features.

1. Put unit tests in a tests/ directory
2. Document how to run tests in a reproducible way on the README
3. Use tox and GitHub Actions to automate running tests on all changes

It's not a common choice to package code in a windows-only executable. Most scientists are using Mac or Linux, so this makes it difficult for those users to use your code. Further, there are pretty big security concerns for downloading some exe off of the internet and running it, so I would say this is not the way you should be distributing your code. See #1 and #2 for a better alternative.

Several files seem to be committed to the repository that make navigating it confusing. Please delete the following directories and add them to a .gitignore file so they don't get accidentally committed again. You can make a base .gitignore file with https://www.toptal.com/developers/gitignore/. Here's a good starting point: https://www.toptal.com/developers/gitignore/api/python,intellij,pycharm,jupyternotebooks,windows,macos,vim

• the dist/ directory should be deleted (in general, build artifacts get sent to places like PyPI, not kept in version control)
• same for src/sbml2hyb.egg-info
• src/__init__.py is not necessary. You only need this inside src/sbml2hyb/ and sub-modules
• There appear to be duplicate python files inside windows_exe/ of the packaged files.

How was the executable https://github.com/rs-costa/sbml2hyb/blob/main/SBML2HYB.exe generated? The code that creates this should be checked in to this repository and documented for rebuilding.