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

test error for order of printed argument with python 3.11 #3750

Closed
Remi-Gau opened this issue Jun 13, 2023 · 7 comments · Fixed by #3751 or #3766
Closed

test error for order of printed argument with python 3.11 #3750

Remi-Gau opened this issue Jun 13, 2023 · 7 comments · Fixed by #3751 or #3766
Assignees
Labels
Effort: low The issue is likely to require a small amount of work (less than a few hours). Maintenance This issue is related to maintenance work. Priority: high The task is urgent and needs to be addressed as soon as possible.

Comments

@Remi-Gau
Copy link
Collaborator

See in a few runs of CI

For example

 FAILED nilearn/plotting/tests/test_matrix_plotting.py::test_sanitize_reorder_error[None] - AssertionError: Regex pattern did not match.
 Regex: "Parameter reorder needs to be one of {False, True, 'complete', 'single', 'average'}"
 Input: "Parameter reorder needs to be one of {False, True, 'single', 'complete', 'average'}."
FAILED nilearn/plotting/tests/test_matrix_plotting.py::test_sanitize_reorder_error[foo] - AssertionError: Regex pattern did not match.
 Regex: "Parameter reorder needs to be one of {False, True, 'complete', 'single', 'average'}"
 Input: "Parameter reorder needs to be one of {False, True, 'single', 'complete', 'average'}."
FAILED nilearn/plotting/tests/test_matrix_plotting.py::test_sanitize_reorder_error[2] - AssertionError: Regex pattern did not match.
 Regex: "Parameter reorder needs to be one of {False, True, 'complete', 'single', 'average'}"
 Input: "Parameter reorder needs to be one of {False, True, 'single', 'complete', 'average'}."
= 3 failed, 2587 passed, 7 skipped, 2 xfailed, 1311 warnings in 1693.15s (0:28:13) =
@Remi-Gau Remi-Gau self-assigned this Jun 13, 2023
@Remi-Gau
Copy link
Collaborator Author

@ymzayek : I will have a look at this.

@Remi-Gau Remi-Gau added Priority: high The task is urgent and needs to be addressed as soon as possible. Maintenance This issue is related to maintenance work. Effort: low The issue is likely to require a small amount of work (less than a few hours). labels Jun 13, 2023
@Remi-Gau
Copy link
Collaborator Author

This is a zombie "bug" 🧟 (it comes back from the dead) or a unsinkable rubber bug (it keeps coming back up).

https://github.com/nilearn/nilearn/actions/runs/5269060267/jobs/9526640379#step:8:14682

@Remi-Gau Remi-Gau reopened this Jun 14, 2023
@Remi-Gau Remi-Gau changed the title test error on ubuntu with python 3.11 test error for order of printed argument with python 3.11 Jun 15, 2023
@ymzayek
Copy link
Member

ymzayek commented Jun 15, 2023

Thought: do we have to treat VALID_REORDER_ARGS (and therefore also VALID_REORDER_VALUES) variable as a set? Can't it be a list or tuple? Can we mimick what the function above that (_sanitize_tri) is doing?

@Remi-Gau
Copy link
Collaborator Author

Pretty sure that this failure is now just due to some regular expression special character escaping so I am implementing your suggestion of using re.escape from #3751.
Will change it to a set so things are consistent within the module: you are correct that it may be confusing otherwise (Future us: "why tuple here and set there?")

@ymzayek
Copy link
Member

ymzayek commented Jun 15, 2023

Yea it's true that in any case you have to escape (e.g. escape the brackets for a list). I think re.escape is good as we want to have an exact match but if it's not working for whatever reason let's just go with a regex matching the first part of that warning which is what you initially implemented

@Remi-Gau
Copy link
Collaborator Author

OK: if I see that test fail one more time I will go with this. Sounds way more pragmatic.

@Remi-Gau
Copy link
Collaborator Author

OK pragmatism will prevail...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Effort: low The issue is likely to require a small amount of work (less than a few hours). Maintenance This issue is related to maintenance work. Priority: high The task is urgent and needs to be addressed as soon as possible.
Projects
None yet
2 participants