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

[MAINT] add script to check duration of test suite #4231

Merged
merged 16 commits into from Mar 5, 2024

Conversation

Remi-Gau
Copy link
Collaborator

@Remi-Gau Remi-Gau commented Jan 17, 2024

Changes proposed in this pull request:

  • add script to collect duration of runs of the test suite in github action and plot the results it over time
  • mostly for monitoring
  • does not go too far back as it as it tracks the workflow that uses tox that is fairly new

Copy link
Contributor

👋 @Remi-Gau Thanks for creating a PR!

Until this PR is ready for review, you can include the [WIP] tag in its title, or leave it as a github draft.

Please make sure it is compliant with our contributing guidelines. In particular, be sure it checks the boxes listed below.

  • PR has an interpretable title.
  • PR links to Github issue with mention Closes #XXXX (see our documentation on PR structure)
  • Code is PEP8-compliant (see our documentation on coding style)
  • Changelog or what's new entry in doc/changes/latest.rst (see our documentation on PR structure)

For new features:

  • There is at least one unit test per new function / class (see our documentation on testing)
  • The new feature is demoed in at least one relevant example.

For bug fixes:

  • There is at least one test that would fail under the original bug conditions.

We will review it as quick as possible, feel free to ping us with questions if needed.

Copy link

codecov bot commented Jan 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.57%. Comparing base (29ae5f1) to head (28148bc).
Report is 24 commits behind head on main.

❗ Current head 28148bc differs from pull request most recent head 5f4b746. Consider uploading reports for the commit 5f4b746 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4231      +/-   ##
==========================================
+ Coverage   92.06%   92.57%   +0.51%     
==========================================
  Files         144      144              
  Lines       16419    18145    +1726     
  Branches     3434     4025     +591     
==========================================
+ Hits        15116    16798    +1682     
- Misses        761      782      +21     
- Partials      542      565      +23     
Flag Coverage Δ
macos-latest_3.10_test_plotting 92.33% <ø> (?)
macos-latest_3.11_test_plotting 92.33% <ø> (?)
macos-latest_3.12_test_plotting 92.33% <ø> (?)
macos-latest_3.8_test_plotting 92.28% <ø> (?)
macos-latest_3.9_test_plotting 92.29% <ø> (?)
ubuntu-latest_3.10_test_plotting 92.33% <ø> (+0.47%) ⬆️
ubuntu-latest_3.11_test_plotting 92.33% <ø> (+0.47%) ⬆️
ubuntu-latest_3.12_test_plotting 92.33% <ø> (?)
ubuntu-latest_3.12_test_pre 92.33% <ø> (?)
ubuntu-latest_3.8_test_min 66.02% <ø> (-2.93%) ⬇️
ubuntu-latest_3.8_test_plot_min 92.08% <ø> (+0.51%) ⬆️
ubuntu-latest_3.8_test_plotting 92.28% <ø> (+0.46%) ⬆️
ubuntu-latest_3.9_test_plotting 92.29% <ø> (+0.47%) ⬆️
windows-latest_3.10_test_plotting 92.31% <ø> (?)
windows-latest_3.11_test_plotting 92.31% <ø> (+0.47%) ⬆️
windows-latest_3.12_test_plotting 92.31% <ø> (?)
windows-latest_3.8_test_plotting 92.26% <ø> (+0.47%) ⬆️
windows-latest_3.9_test_plotting 92.27% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@Remi-Gau
Copy link
Collaborator Author

output example

newplot(1)

@Remi-Gau
Copy link
Collaborator Author

does not have to go into this repo but handy to have around

@Remi-Gau Remi-Gau marked this pull request as ready for review January 18, 2024 07:58
@Remi-Gau Remi-Gau changed the title [WIP][MAINT] add script to check duration of test suite [MAINT] add script to check duration of test suite Jan 18, 2024
@bthirion
Copy link
Member

output example

newplot(1)

Thx. Not that the use of a unqiue color for different curves makes it barely usable.

@bthirion
Copy link
Member

OK, you will then create the Github action, I guess ?

Copy link
Member

@bthirion bthirion left a comment

Choose a reason for hiding this comment

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

Could not spot any issue.

@Remi-Gau
Copy link
Collaborator Author

OK, you will then create the Github action, I guess ?

yeah to run on command and every month or so.

@Remi-Gau
Copy link
Collaborator Author

will update graph and add this to CI to help automate it

@Remi-Gau
Copy link
Collaborator Author

Not that the use of a unqiue color for different curves makes it barely usable.

improved this a bit:

  • color = python version
  • dash = OS
  • marker = dependencies

test_runs_timing

@Remi-Gau
Copy link
Collaborator Author

OK, you will then create the Github action, I guess ?

This is done

Copy link
Member

@bthirion bthirion left a comment

Choose a reason for hiding this comment

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

LGTM, thx.

@bthirion
Copy link
Member

Happy to hear about other's opinion on this feature.

@Remi-Gau
Copy link
Collaborator Author

Remi-Gau commented Feb 1, 2024

Happy to hear about other's opinion on this feature.

As this is not urgent I will wait for a couple of weeks before merging.

@Remi-Gau Remi-Gau marked this pull request as draft February 1, 2024 06:54
@Remi-Gau Remi-Gau marked this pull request as ready for review February 26, 2024 12:52
@Remi-Gau
Copy link
Collaborator Author

Unless I hear anything otherwise I will merge this next week.

@Remi-Gau Remi-Gau closed this Feb 29, 2024
@Remi-Gau Remi-Gau deleted the test_speed branch February 29, 2024 07:56
@Remi-Gau Remi-Gau restored the test_speed branch February 29, 2024 07:58
@Remi-Gau Remi-Gau reopened this Feb 29, 2024
return next(
(
version
for version in ["3.8", "3.9", "3.10", "3.11", "3.12"]
Copy link
Member

Choose a reason for hiding this comment

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

Is there a way we could keep this in-sync with the Python versions we support, without having to remember to update it here ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Cannot think of an easy way... Yet.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Could not find an easy way to do this, so for now I at least extract this list into a constant at the top of the script that should make it easier to find and modify.

Comment on lines +18 to +38
This may require a github token to work
(if you need to make a lot of call to the GitHub API).

You can get a github token at:
https://github.com/settings/tokens

You can either:

- save the github token in a file and modify the script
(see the variable USERNAME and TOKEN_FILE below).
This is can be useful if you want to run the script locally.

- pass the token directly to the script as an argument.
This is the way to do it when using this script in continuous integration.

USAGE
-----

.. code-block:: bash

python maint_tools/check_gha_workflow.py $GITHUB_TOKEN
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@emdupre
does this seem more clear?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, thanks !

@Remi-Gau Remi-Gau merged commit 9d2090f into nilearn:main Mar 5, 2024
12 of 31 checks passed
@Remi-Gau Remi-Gau deleted the test_speed branch March 5, 2024 08:26
Remi-Gau added a commit to Remi-Gau/nilearn that referenced this pull request Mar 5, 2024
* add script to check duration of test suite

* add comment

* comment and refactor

* typo

* improve paging

* add workflow

* Update maint_tools/requirements.txt

* fail on bad request

* upload artefacts

* upload artefacts

* remove running on PR

* Apply suggestions from code review

Co-authored-by: Elizabeth DuPre <emd222@cornell.edu>

* update doc string

* update doc string

* refactor

---------

Co-authored-by: Elizabeth DuPre <emd222@cornell.edu>
Remi-Gau added a commit that referenced this pull request Mar 9, 2024
…ap with masker report images (#4308)

* extract table folding and overlapping

* update changelog

* Update doc/changes/latest.rst

Co-authored-by: bthirion <bertrand.thirion@inria.fr>

* FIX] fixes several plotting details in glm reports (#4266)

* fix several issues in glm reports

* further fixes

* change cmap for glass brain

* fix remaining issues

* [full doc] request full build

* update changelog

* [full doc] request full build

* [FIX] force views in surface plotting to be pairs of `int` or `float` when not a `string` (#4297)

* for views to be pairs of int or float when not a string

* update changelog

* [FIX] fix cut position in nifti maps maskers (#4304)

* fix cut position in nifti maps maskers

* update changelog

* [FIX] remove conftest for externals (#4306)

* remove conftest tempita

* ignore tempita in pyproject.toml

* [MAINT] add script to check duration of test suite (#4231)

* add script to check duration of test suite

* add comment

* comment and refactor

* typo

* improve paging

* add workflow

* Update maint_tools/requirements.txt

* fail on bad request

* upload artefacts

* upload artefacts

* remove running on PR

* Apply suggestions from code review

Co-authored-by: Elizabeth DuPre <emd222@cornell.edu>

* update doc string

* update doc string

* refactor

---------

Co-authored-by: Elizabeth DuPre <emd222@cornell.edu>

* [DOC] display all masker reports in the doc (#4295)

* display masker reports in doc

* update changelog

* refactor

* add notebooks

* clear cells

* [full doc] request full build

* add link to examples

* [full doc] request full build

* fix doc build

* [full doc] request full build

* [full doc] request full build

---------

Co-authored-by: bthirion <bertrand.thirion@inria.fr>
Co-authored-by: Elizabeth DuPre <emd222@cornell.edu>
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.

None yet

3 participants