Skip to content

Update Python, improve CI workflows, and enhance testing coverage - #246

Open
pgleeson wants to merge 24 commits into
developmentfrom
ow-githubactions
Open

pgleeson wants to merge 24 commits into
developmentfrom
ow-githubactions

Conversation

@pgleeson

Copy link
Copy Markdown
Member

Adds Github action runs for most of the standard examples, checking that they produce expected output.
Fixes issues with configurations one_spring_test, test_energy, v_test_liquid
Adds SiberneticReplay.py, which can reload sibernetic simulations & visualise them for replay using pyvista. Try:

pip install -r requirements.txt
./test.sh -quick
python SiberneticReplay simulations/test_opencl_demo1

@skhayrulin skhayrulin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pgleeson I’ve looked at the code, and the main comments concern style, though I’m not sure how critical that is.

- name: Run tests
run: |

export NEURON_HOME=/home/runner/.local

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it ok that we use full path to runner, I mean that could we be sure that folder /home/runner/.local is exist?

Comment thread tests/test_c2fw.py
import sys
import os

from test_utils import restructure_output_for_omv

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that I understand for what this file for, but don't you think to use pytest for test run, and collecting information about which tests was ok and which is failed?

Comment thread plot_positions.py
if num_plotted_frames % 3 == 1:
time = (
"%sms" % t_ms
if not t_ms == int(t_ms)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think here is better compare with != operator?

Comment thread plot_positions.py
index+=1

print_("Loaded: %s points from %s, showing %s points in %i plots"%(index,pos_file_name,points_plotted,num_plotted_frames))
print_(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use f string for python it's really more clear

f"Loaded: {index} points from {pos_file_name}, showing {points_plotted} points in {num_plotted_frames} plots"

Comment thread plot_positions.py

count += 1

import matplotlib.pyplot as plt

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

usually it's not good to add imports to function

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants