Hi. We are members of the Research Software Group within the Advanced Research Computing team at the University of Birmingham and, as part of an on-going project, we were requested to review the MCPoly repository and report any issues. Please find below a list of the things that we've identified:

• It is not clear how to start the application. Based on the screenshots we found that you can run

   from MCPoly import vis
   vis()
  

  but this doesn't work from a python shell, where we get:

       Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "PATHTO/MCPoly/vis/vis.py", line 46, in vis
   gui1()
  File "PATHTO/MCPoly/vis/../orcaset/gui.py", line 289, in gui
   display(widgets.HBox([frame1,output,output2]))
   NameError: name 'display' is not defined
  

  Maybe we are missing a from IPython.display import display or this is not the intended way to use the app. Some instructions in the README would be useful to the user.
• From the same screenshots it seems that you are using the library from within a jupyter notebook. However, jupyter was not installed automatically from the requirements file as it is not included and also it is not a dependency of the other packages. You might want to include jupyterlab in the requirements.txt file, depending on how you want users to use the software.
• The MCPoly release link from the Installation section doesn't work. It seems that you are missing an "s" in the end (should be releases not release). Even when you add it though, the releases is empty.
• The How to cite just gives a link to the repo with no instructions on how to actually cite the repo and more info about the authors etc. Maybe adding a CITATION.cff file will help those who want to properly cite the software.
• The MANIFEST.in file contains include src/MCPoly/* but there is no src/ directory in the repo. This is probably a remnant from older versions.
• Code doesn't adhere to any specific style guide (see PEP8). For example, there are various if sth == True instead of the proper for Python if sth, there are no spaces after commas, no spaces before and after comparison operators, long lines, etc. There are many code formatters and linters that can help with this, and the most popular for Python are flake8, black and pylint.
• pytest is also not in the requirements file. You might want to create a requirements-test.txt file and not add it in the main requirements file.
• The only way to run the tests is from the parent of the root repo directory, with pytest MCPoly/tests because of various hardcoded relative paths in the form ./MCPoly/tests/.... Also, the tests return a few deprecation warnings.
• A CI pipeline, using GitHub Actions, that will automatically run the code formatters/linters and the tests, for the Pull Requests or commits in the main branch, would really help with further developing the applications and more easily identifying bugs.
• tests/__pycache__ shouldn't be committed and pushed to git. It will help a lot if you include a .gitignore file (one that works well for Python projects can be found here).
• The version on PyPI do not match those on GitHub. For example, currently the latest version on PyPI is 0.5.1.3, released on 29/6/2023 but there is no branch nor release on GitHub that matches this version. Actually, there are no releases at all on GitHub. There are three branches though, a main, a 0.4.0 and a 0.5.0. However, those don't seem to match any of the versions on PyPI either. Furthermore, GitHub was last edited on 2/6/2023 but PyPI get continuously updated and it is not clear where those version are coming from. Finally, the 0.5.0 branch on GitHub is 11 commits ahead and 14 commits behind main.
• There is a bug with how you call the orca executable. In the MCPoly/MCPoly/orcaset/orca.py module, in the definition of the orca() function there is the orcaloc input argument with a default value of './'. Later in the code if orcaloc=='./' then you call orca with its input arguments. However, since there is no ./ before the executable name, this means that the system launches the orca that is found in $PATH, which is not the intended behaviour. Furthermore, orca is also the screen reader in many Linux distributions that use the Gnome desktop environment, which means that this is launched instead of the orca from https://www.orcasoftware.de (this is actually what happened in one of the systems that we've tested the code from).