JOSS Review : general comments

Some of the items may be repeated in other issues. Just wanted to be sure we don't forget anything from https://github.com/openjournals/joss-reviews/issues/2408#issuecomment-655272567.

[x] Out of the box, running tests in unittests/ folder from PyCharm (on Windows) worked for me. Test_toy took some time to run but maybe that's expected => Check and see if one of the tests is too long and change accordingly
[x] Related to #42, fix the whole benchmarks item
Out of the box, all tests in benchmarks/ fail. All errors, except the one in test_ortools, are the same:

with open(path + instance_name) as fp:
E FileNotFoundError: [Errno 2] No such file or directory: '../examples/benchmarks/data/P-n16-k8.vrp'
....\examples\benchmarks\cvrp_augerat.py:53: FileNotFoundError

You seem to have an issue with setting path variables. Please check. test_ortools error is

from ortools.data import (
E ModuleNotFoundError: No module named 'ortools'

Again, a similar issue. It can't find the ortools folder which is under examples/

Running mdvrp_cordeau.py fails with

raise KeyError("Node %s missing from initial solution." % v)
KeyError: 'Node 52 missing from initial solution.'

[x] I suggest renaming the "ortools" folder. The current import statements read as if you have a dependency on the ortools library. It's not obvious that it's just a folder in the upper level.
[x] Some examples are using numpy.matrix(). It is not recommended to use. Please consider regular numpy ndarrays. => This is NetworkX dependent, needs investigation
[x] Some clean up
- There is a lot of mingling with path variable to append sys.path for folders. I don't think this is necessary and complicates the code/usage --since one now needs to track what's in the path and what's not. I don't see anything special in your library structure that would require altering with path variables. Cannot we resolve these import issues without hacking into the path?
- In test_toy.py, remove "from pytest import raises" --not used. same for test_greedy
In cvrp_augeratpy, remove "from vrpy.main import VehicleRoutingProblem" --not used
In benchmarks folder, some files have a main() function, some files don't. Why?
[x] In examples/ folder, you typically print the best value and best route. Please also ASSERT them. I can run them, but I don't know if the solution is correct or not.
[x] why is the separation between tests and examples. Why not add these examples to test and assert them.
[x] is there a way to turn off logging from the underlying solvers like CBC etc? CBC should have some verbosity flag. There are a lot of printouts that are not directly related to this library, and might be confusing for the uninterested user.
[x] examples.ipync is nice.. but please add ASSERTs along with prints() so that we can verify correctness.
[x] You might want to add LICENSE information as a header on top of every source code file. Maybe also add License section to README?
[x] Thinking out loud comment: if you rename main.py to vrp or vehicle_routing, then the imports would read:

from vrpy.vrp import VehicleRoutingProblem
from vrpy.vehicle_routing import VehicleRoutingProblem

which might read a bit more relevant than importing "main"

Kuifje02 / vrpy

JOSS Review : general comments #53