Skip to content
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

setting up CI for the repo #33

Merged
merged 17 commits into from
Dec 4, 2024
Merged

setting up CI for the repo #33

merged 17 commits into from
Dec 4, 2024

Conversation

grg2rsr
Copy link
Contributor

@grg2rsr grg2rsr commented Dec 2, 2024

No description provided.

@grg2rsr grg2rsr requested a review from oliche December 2, 2024 22:09
@grg2rsr
Copy link
Contributor Author

grg2rsr commented Dec 2, 2024

#11

@grg2rsr grg2rsr changed the title making test_plots.py independent from ONE setting up CI for the repo Dec 2, 2024
@grg2rsr
Copy link
Contributor Author

grg2rsr commented Dec 3, 2024

in the process of removing ONE from test_plots.py, I had to change the hard coded column key stimOnTrigger_times to stimOn_times. I used the same files as specified in the eid and grabbed the data from the mainenlab server, and stimOnTrigger_times is not present in trials.table, but stimOn_times is and tests are passing. I wonder how this came about, were there different extraction runs, and @GaelleChapuis has locally a different dataset then what is stored on the server? solved.
Additionally, I think it might be better to have the tests based on data from Caronlina, as they were run with an extractor that we are going to re-use in the future.

@grg2rsr grg2rsr requested a review from bimac December 3, 2024 10:23
Copy link

@bimac bimac left a comment

Choose a reason for hiding this comment

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

All good if the CI is running - which it seems to do.

Just a few opinions:

  • Use actions/setup-python@v5
  • Caching makes GH workflows a lot snappier as you don't have to set up the environment from scratch each time, see https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/caching-dependencies-to-speed-up-workflows
  • I personally use PDM as a package manager - it simplifies a few things about the GH workflow (especially when it comes to publishing at some point) - happy to walk you through it if you're interested
  • it might be useful to save the versions of dependencies as a workflow artifact (requirements-frozen.txt)
  • how about adding coverage? you could use the pytest-cov plugin for that ...

@oliche oliche merged commit f0280f7 into main Dec 4, 2024
5 of 7 checks passed
@oliche oliche deleted the CI_setup branch December 4, 2024 11:44
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.

3 participants