search

OpenSourceBrain / AllenInstituteNeuroML

Example files to test interaction between models developed at the Allen Brain Institute and NeuroML
http://www.opensourcebrain.org/projects/alleninstituteneuroml
Other
16 stars 10 forks source link

Plot data fix/clean up and github actions for #13 #14. #16

Closed anujanegi closed 2 years ago

anujanegi commented 2 years ago

What is the point of Fig_TEMPLATE.html, as we don't really use Fig.html created by it and only use CellInfo_TEMPLATE.html.

Is it just redundant and should be removed? @pgleeson

pgleeson commented 2 years ago

@anujanegi I'll work first at getting this merged, as there are overlapping files with #15, and also there there are multiple commits with zero changes to image files, as well as the generated analysis files, https://github.com/OpenSourceBrain/AllenInstituteNeuroML/pull/15/files#diff-19e5412a4fa6fbcafc074d862e6240fcb1bf50a489f1c8029b3a107954da9d6f are committed. These don't need to be committed, as the contents arren't changed, just different date and different percision in the datapoints.

I've enabled actions on your PRs. See https://github.com/OpenSourceBrain/AllenInstituteNeuroML/actions. The test on python 3.9 failed with an issue related to the allensdk: https://github.com/OpenSourceBrain/AllenInstituteNeuroML/runs/6966489302?check_suite_focus=true#step:9:100

Py3.9 doesn't seem to be supported there, so can you change the test to run on 3.7 and 3.8?

Re Fig_TEMPLATE.html, it was for testing the generation of a figure for the OSB paper. The generated Figure.html didn't need to be committed. You can remove the template and remove the lines in data_summary.py that use it.

anujanegi commented 2 years ago

@pgleeson Thanks for the feedback! Will make sure to not push the analysis files from here on. I think I messed up with the overlapping commits, hence merging this with #15.

pgleeson commented 2 years ago

@anujanegi Thanks, but it may have been better to keep the other PR and close this one as this has 48 modified files...

pgleeson commented 2 years ago

FYI @anujanegi the tests on this PR are failing here: https://github.com/OpenSourceBrain/AllenInstituteNeuroML/runs/7009024415?check_suite_focus=true

Try this line (in the other PR) to set the NEURON_HOME: https://github.com/OpenSourceBrain/NetPyNEShowcase/blob/2fc7082bf263a543149e79d3c7be80f70ee8ab1c/.github/workflows/non-omv.yml#L37

anujanegi commented 2 years ago

@pgleeson Tried to rebase to remove analysis files that I committed. Some conflict resolutions are needed, but fewer file changes now.

pgleeson commented 2 years ago

@anujanegi I think the simplest thing in this case is to create a new branch from this repo's master, and add any changed files there, with a fresh PR.