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

Fix the Github Action CI for Python tests #445

Merged
merged 9 commits into from
Oct 9, 2024

Conversation

remrama
Copy link
Contributor

@remrama remrama commented Oct 8, 2024

This PR corrects a typo in the Python tests Github Action CI yaml file that was preventing the tests from running on PRs. The pingouin master branch was recently renamed from master to main, and now the yaml file reflects that. This was identified in a comment in #443.

Copy link

codecov bot commented Oct 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (main@c93ec4b). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #445   +/-   ##
=======================================
  Coverage        ?   98.54%           
=======================================
  Files           ?       19           
  Lines           ?     3360           
  Branches        ?      547           
=======================================
  Hits            ?     3311           
  Misses          ?       26           
  Partials        ?       23           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@remrama
Copy link
Contributor Author

remrama commented Oct 8, 2024

Ok, took a few adjustments but all tests are passing now! Sorry for all the pushes, I still am not aware of a way to test GH Actions locally... I am pushing just to see if something works 😕 If anyone has advice on how to test locally and avoid this approach, please let me know!

Updated list of changes

I had to bump a few action versions up where the older ones caused an issue, and updated the docs requirements installation to take advantage of the modern python packaging layout that happened recently in #406.

  • [master, develop] --> [main]
  • actions/upload-artifact@v2 --> actions/upload-artifact@v4 (see migration guide)
  • actions/checkout@v1 --> actions/checkout@v4
  • actions/setup-python@v1 --> actions/setup-python@v5
  • pip install docs requirements from .toml file with pip install .[docs]
  • Pingouin currently uses codecov/codecov-action@v1 but latest is v4. The current version is not leading to any errors, but it leads to warnings about node usage. If you want, I can also try bumping the codevoc action while I'm here. I might run into some token access issues but could play with it.
  • Is there supposed to be a download-artifact action in the Python tests workflow? The Pingouin documentation page has an "Inspect on Github" section with instructions to view the artifact but they don't correspond to what I see on my end (i.e., I don't see downloadable artifacts to inspect). Maybe I'm missing something here? Also that section refers to tests on Python version 3.8 which Pinguoin no longer runs (I suspect this should be 3.9 since that is the version that builds docs during tests).

Any thoughts on those last 2 hanging points?

@raphaelvallat raphaelvallat self-requested a review October 8, 2024 18:47
@raphaelvallat raphaelvallat added bug 💥 Something isn't working URGENT ⚠️ To fix ASAP IMPORTANT❗ labels Oct 8, 2024
@raphaelvallat
Copy link
Owner

raphaelvallat commented Oct 8, 2024

You're on a roll, thank you!!

I think they changed the interface for downloading artefacts. The artefacts can be found at the bottom of the GH Actions Summary page:

image

You can also find the url in the raw log:

image

The Contributing page will need to be updated, though it's a low priority task since I'm pretty sure no one has ever used that feature haha

Please do try to update codecov, and let me know if you run into any issues.

.github/workflows/python_tests.yml Outdated Show resolved Hide resolved
@remrama
Copy link
Contributor Author

remrama commented Oct 8, 2024

Alright that should be good @raphaelvallat! I also removed the platform and python version from the docs artifact. I added it before, but it's only necessary if you're uploading artifacts from multiple platforms/versions. That's not the case here, so it's cleaner as it was without.

I don't see the same web interface for artifacts as you do in the screenshot, but I found the URL in the logs as you suggested (thanks for the pics, very helpful). Agreed, not a big deal for now.

Copy link
Owner

@raphaelvallat raphaelvallat left a comment

Choose a reason for hiding this comment

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

Amazing thank you! Merging now

@raphaelvallat raphaelvallat merged commit a3fa6fb into raphaelvallat:main Oct 9, 2024
12 checks passed
@remrama remrama deleted the pytest_github_actions branch October 9, 2024 13:52
raphaelvallat pushed a commit that referenced this pull request Dec 8, 2024
* CI95% --> CI95

* p-* --> p_* (also U-* and W-*)

* CI[97.5%] --> CI97.5

* mean(A) --> mean_A (also std)

* T-test --> T_test (index)

* effect sizes

* updated notebooks

* Fix the Github Action CI for Python tests (#445)

* GH Action on main branch instead of master and develop

* bump actions/upload-artifact@v2 to v4

* install doc requirements from .toml

* bump actions/checkout@v2 --> v4

* bump actions/setup-python@v1 --> v5

* move pip-install-docs back to only run during docs build

* typo 3.8 --> 3.9

* bump codecov/codecov-action@v1 --> v4

* remove platform specification from docs-artifact

* CI95% --> CI95

* p-* --> p_* (also U-* and W-*)

* CI[97.5%] --> CI97.5

* mean(A) --> mean_A (also std)

* T-test --> T_test (index)

* effect sizes

* updated notebooks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 💥 Something isn't working IMPORTANT❗ URGENT ⚠️ To fix ASAP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants