-
Notifications
You must be signed in to change notification settings - Fork 124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix travis build and tests with mock data #571
Conversation
Nest 2.14 is tested on python 3.4, and the travis images to not contain py3.5 either. Removing it to see if nest builds correctly here.
The errors are related to the mock simulator---probably something trivial. The one here, for example, is only because the variable |
Some tests related to NEST and neuron too are failing. The build bit etc. now works. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks very much for this PR. Unfortunately I had already fixed some of the bugs in my own fork but forgot to push it to upstream.
I would be interested to receive a new PR with the changes to setup.py
env: PYENV=py35 | ||
python: | ||
- 2.7 | ||
- 3.5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to test with an older version of Python 3 (3.3 or 3.4), since HPC environments do not always have recent versions available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yes, of course. I'm not modifying the .travis.yml
file now, so it'll be as you left it.
@@ -5,3 +5,7 @@ numpy>=1.8.2 | |||
quantities>=0.12.1 | |||
lazyarray>=0.3.2 | |||
neo>=0.5.2 | |||
setuptools | |||
matplotlib |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
matplotlib, scipy and even mpi4py are optional, they are not strict requirements
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. I'd done a quick grep of the code and found this:
$ grep -nri "matplotlib" *
doc/pyplots/continuous_time_spiking.py:3:import matplotlib.pyplot as plt
doc/pyplots/reset_example.py:2:import matplotlib.pyplot as plt
doc/pyplots/neo_example.py:2:import matplotlib.pyplot as plt
doc/pyplots/plot_helper.py:5:import matplotlib.pyplot as plt
doc/data_handling.txt:132:Plotting Neo data with Matplotlib, as shown above, can be rather verbose, with
doc/data_handling.txt:137:highly-customized plots you should probably use Matplotlib or some other
doc/developers/contributing.txt:29: * matplotlib
doc/developers/contributing.txt:263:.. _matplotlib: http://matplotlib.sourceforge.net/
doc/conf.py:21: # also, the matplotlib figures need the real modules.
doc/conf.py:55:# 'matplotlib.sphinxext.only_directives',
doc/conf.py:56:# 'matplotlib.sphinxext.plot_directive'
examples/random_distributions.py:7:import matplotlib.pyplot as plt
examples/random_distributions.py:8:import matplotlib.gridspec as gridspec
examples/HH_cond_exp2.py:58: import matplotlib.pyplot as plt
examples/stochastic_synapses.py:7:import matplotlib
examples/stochastic_synapses.py:8:matplotlib.use('Agg')
examples/stochastic_deterministic_comparison.py:7:import matplotlib
examples/stochastic_deterministic_comparison.py:8:matplotlib.use('Agg')
Binary file examples/iaf_sfa_relref/myFigure_expected.pdf matches
examples/stochastic_tsodyksmarkram.py:17:import matplotlib
examples/stochastic_tsodyksmarkram.py:18:matplotlib.use('Agg')
examples/tools/VAbenchmark_graphs.py:23:import matplotlib.pyplot as plt
examples/tools/VAbenchmark_graphs.py:24:import matplotlib.gridspec as gridspec
examples/tools/comparison_plot.py:13:import matplotlib.pyplot as plt
examples/tools/comparison_plot.py:14:import matplotlib.gridspec as gridspec
examples/tools/comparison_plot.py:50: # also consider adding metadata to PNG file - see http://stackoverflow.com/questions/10532614/can-matplotlib-add-metadata-to-saved-figures
examples/tools/plot_results.py:16:import matplotlib.pyplot as plt
examples/Potjans2014/README.txt:90:matplotlib 0.99.1.1, and glob.
examples/Potjans2014/plotting.py:2:import matplotlib
examples/Potjans2014/plotting.py:3:matplotlib.use('Agg')
examples/Potjans2014/plotting.py:4:import matplotlib.pyplot as plt
examples/multiquantal_synapses.py:7:import matplotlib
examples/multiquantal_synapses.py:8:matplotlib.use('Agg')
examples/gif_neuron.py:20:import matplotlib
examples/gif_neuron.py:21:matplotlib.use('Agg')
pyNN/utility/plotting.py:6:figures, it will probably be easier to use matplotlib or another plotting
pyNN/utility/plotting.py:18:import matplotlib.pyplot as plt
pyNN/utility/plotting.py:19:import matplotlib.gridspec as gridspec
pyNN/utility/plotting.py:223: A panel is a Matplotlib Axes or Subplot instance. A data item may be an
pyNN/utility/plotting.py:228: Valid options are any valid Matplotlib formatting options that should be
pyNN/utility/plotting.py:234: a list of dicts containing Matplotlib formatting options, of the
test/benchmarks/plot_figure.py:21:import matplotlib.pyplot as plt
test/benchmarks/plot_figure.py:22:import matplotlib.gridspec as gridspec
test/system/scenarios/test_synapse_types.py:33: import matplotlib.pyplot as plt
test/system/scenarios/test_cell_types.py:28: import matplotlib.pyplot as plt
test/system/scenarios/test_cell_types.py:88: import matplotlib.pyplot as plt
test/system/scenarios/test_cell_types.py:118: import matplotlib.pyplot as plt
test/system/scenarios/test_cell_types.py:170: import matplotlib.pyplot as plt
test/system/scenarios/test_cell_types.py:226: import matplotlib.pyplot as plt
test/system/scenarios/ticket166.py:43: import matplotlib.pyplot as plt
So it seems to be at least imported in a lot of tests, and possibly used in quite a few examples. Should I leave it in the requirements file so that it's available to the tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly, mpi4py
and scipy
are required by a few tests, so I reckon they should be in requirements.txt
if not in setup.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a strong preference for keeping requirements.txt
minimal - only what is needed to install the core functionality.
This preference may be outdated - it is from the time before widespread adoption of wheels, when pip-installing scipy
or matplotlib
would often result in compilation from source, taking 20 minutes or more. This was the case probably only two years ago, although I agree the situation currently is greatly improved.
How about a file requirements_testing.txt
and/or requirements_extras.txt
, and using the tests_require
keyword of setuptools
in setup.py
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yes, that sounds like a good idea. I'll update the PR I just opened.
@@ -108,5 +111,6 @@ def find(self, command): | |||
'Programming Language :: Python :: 3.6', | |||
'Topic :: Scientific/Engineering'], | |||
cmdclass={'build': build}, | |||
install_requires=['numpy>=1.8.2', 'lazyarray>=0.3.2', 'matplotlib', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
matplotlib is not required, it is optional for the core PyNN functionality
Various updates to the travis configurations to get the
install.sh
script to properly install pynn, nest and other deps required to run the tests. The tests still fail, but not because the build requirements cannot be installed. These are the failed tests that one can now work on: