Add Travis CI

[rht]

No description provided.

[rht]

Contains #4, depends on that PR to be merged first. The commit there is included here so that Travis test can run.

[davidrpugh]

@rht Thanks for the PR. Couple of comments about the Travis script. Why only testing against Python 3.5 and Linux OS? Seems quite specific. Default Anaconda Python distributions are Python 3.6 and Python 2.7 (although I would not recommend supporting Python 2.7...but this is up to @x0range and @DavoudTaghawiNejad). I would also recommend testing for Linux and Mac OSX. I am configuring all ESL repos to run static analysis and testing using Codacy so no need to worry about flake8 and coveralls.

Finally, can you explain the purpose of the test? In particular, why is it necessary to introduce the isTravis flag? This introduces a dependency between the core model logic and CI infrastructure which is undesirable. Perhaps if you can explain more about what you wanted to accomplish with the test I can help come up with an alternative solution.

[rht]

Noted. In this case, it was because the requirements.txt install process is
faulty in Python 2.7. I have observed the CI build hangs >20% of the time in
particular when installing Pandas (required for the graphs and GUI's).
For reference for @DavoudTaghawiNejad and @x0range to decide, here is the timeline for most scientific projects by 2020: http://www.python3statement.org/.

I see, I didn't know that Codacy automatically does coverage and lint

Finally, can you explain the purpose of the test?

The intent of the test is to make sure the main simulation and the model code,
start.py runs, as there isn't any existing unittest/integration test yet. I
added isTravis because by default start.py launches a web server for the
graphs visualization. I didn't intend to change this default, but I had to make
the run terminates hence the option. The course of actions are either: 1. add
tests (but this will be delayed by having to write the documentation), 2.
Default to not displaying graphs (I prefer this one), 3. Current method, a
compromise, but complicates the start.py (model and run) file as you said.

[rht]

@davidrpugh I added 3.6 and macOS.

[davidrpugh]

@rht An alternative to using pip would be to add a environment.yml file and use conda which comes pre-installed with the Anaconda Python distribution. When I gave a talk at the SciPy conference a couple of years ago it seemed pretty clear to me that conda as a package management tool was preferred to pip (at least within the Scientific Python community). Are you familiar with conda?

I now understand the purpose of the isTravis flag. Perhaps @DavoudTaghawiNejad could add a headless option to ABCE so that users could run a simulation without launching the GUI? This would solve our problem as we could then run the simulation without having to add a new flag to the constructor.

[rht]

Hmmm, years ago, I found conda as being freemium with a proprietary license, so I didn't opt for it (lest the repeat of MATLAB ecosystem happening again). I know it solved the installation setup headache, but I already had a working setup for my libraries. This is just my personal preference. In one way or another, defaulting to pip here is more agnostic, and there is no inevitable 2020 deadline as in the Python2 case.

[rht]

Here is the line where the graphs are automatically produced and the web server launched. I'd recommend to having the data analysis residing in a Jupyter notebook actually. This offloads the graph-visualization-in-a-web-server development effort to another library.

[rht]

(e.g. https://github.com/EconomicSL/auctions-simulation-example/blob/master/periodic-double-auction-analytics.ipynb) Also, Jupyter notebooks, if versioned, can be cited from its hash while web server graphs can't. The @gui will have its purpose for interactive exploration.

[rht]

Actually, the interact/manipulate GUI parameters can be further offloaded to Jupyter (ref: https://ipywidgets.readthedocs.io/en/latest/examples/Using%20Interact.html). So as to approach http://distill.pub/2017/momentum/.

[x0range]

Maybe I am misunderstanding something. To me, it seems, this will fail with an error when not run by travis. Boolean isTravis is undeclared in that case. Maybe put this in a try/except clause. But this also seems to not be the proper way to do it.

[rht]

@x0range, Boolean isTravis is wired from the parser.add_argument with a default to be False. I think in these cases, a default argument is cleaner to use than try-except. C struct and Protobuf have default values, for instance. Python dictionary has .get method with default value.

[davidrpugh]

@rht I agree with @x0range this is not the proper way to handle this issue. It should be possible to run a simulation without a GUI using ABCE. If it is not possible, then this is an issue for @DavoudTaghawiNejad to fix upstream in ABCE. Until then we can focus on testing logic that does not require ABCE.

Come to think of it, @DavoudTaghawiNejad mentioned that he has tests running on Travis, how is he solving this problem?

[x0range]

@davidrpugh It is possible to switch off the GUI. I am doing that in the branches I am developing in https://github.com/x0range/isle using a command line switch taken in with sys.argv. But in my opinion it makes sense to have the GUI switched on by default, especially if we plan to at some point deploy this somewhere and make it available to non-programmers.

[x0range]

@rht I still do not understand. If you run this from the terminal, it results in NameError: name 'isTravis' is not defined. This would be expected, since isTravis is not declared. The only two solutions I can see are to put it in a try-except clause or to switch the GUI off by default (as suggested by @davidrpugh)

[davidrpugh]

@x0range Good to know about the command line switch. This should solve the problem for CI purposes as we should be able to pass the same parameter as part of the command line script on Travis. In general boolean arguments should default to False. This used to be part of the Google Python style guide but I can not find the link at present. Will try to find a reference for this.

[rht]

!!! Sorry, I was late to see the discussion. The test suites in ABCE are currently unittests[1] and have yet to cover for the GUI area of the code. The boolean in argparse should have False as a default as can be seen in:

parser.add_argument('--travis', dest='travis', action='store_true',
                        default=False,
                        help='Flag to disable gui when run in a Travis environment')

@x0range I wired it at main(simulation_parameters, args.travis); I tested on my machine (uname -o: GNU/Linux, Python 3.6.1, this PR) and it launched the GUI. Unless there is an anomaly in the system you are running, it should have run. Yes, the main function has isTravis as a required argument.

For Protobuf, the default value for bools is false[2]. And yes, in general as well -- a switch is off/0 by default for safety purpose in EE, isn't it?

[1] https://github.com/AB-CE/abce/blob/c93db46ab313f54d6d626725952beac34504fa67/.travis.yml#L47
[2] https://developers.google.com/protocol-buffers/docs/proto

[davidrpugh]

@rht For as long as I have been aware of conda it has been open-source BSD 3-clause license. I use conda where ever possible and pip only when necessary. I don't think that this is a big issue however and only mentioned it because you raised issues with pip.

I completely agree that data visualization and analytics should not be part of any core ABM simulation library; decisions about data analytics and visualization should be left up to the user or developed separately from the main simulation library. However these are questions/recommendations for @DavoudTaghawiNejad since ABCE is his project. Might consider opening issues on that repo?

[rht]

Note: I rebased and updated the travis.yml with the most recent version of Xcode on os: osx, which has 3.7 by default.

[rht]

I think the switch for this repo has been disabled.