Skip to content

Conversation

lesteve
Copy link
Member

@lesteve lesteve commented Aug 27, 2025

Part of #30007

  • going for Python 3.14 seems like the best bet. I get segmentation faults from time to time on Python 3.13 when running locally. They seem related to warnings looking at the Python traceback.

Edit: scipy free-threaded Python 3.14 package was added in conda-forge so the following 2 points are not a problem anymore:

Copy link

github-actions bot commented Aug 27, 2025

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: 177ac96. Link to the linter CI: here

@lesteve lesteve force-pushed the free-threaded-pytest-run-parallel branch from 5e0ee8f to 47b7d33 Compare August 27, 2025 10:43
@@ -1329,18 +1329,19 @@ def test_vectorizer_stop_words_inconsistent():
vec.fit_transform(["hello world"])
# reset stop word validation
del vec._stop_words_id
assert _check_stop_words_consistency(vec) is False
with pytest.warns(UserWarning, match=message):
Copy link
Member Author

@lesteve lesteve Sep 1, 2025

Choose a reason for hiding this comment

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

This is the actual fix, see previous discussion in #32023 (comment).

My guess right now is that the first call _check_stop_words_consistency was raising a warning and that the next pytest.warns was sometimes failing because of the default warnings once strategy plus some unfavourable thread ordering. By putting this inside pytest.warns we avoid the side-effect and the second warning is always issued.

warnings.simplefilter("error", UserWarning)
vec.fit_transform(["hello world"])
assert _check_stop_words_consistency(vec) is None
# Only one warning per stop list
Copy link
Member Author

Choose a reason for hiding this comment

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

This was an oversight. This puts this block inside the for loop. Previously, this block was not inside the for loop so only the last element of the loop was tested ...

# if the parameter is deprecated, don't show it
continue
finally:
warnings.filters.pop(0)
Copy link
Member Author

Choose a reason for hiding this comment

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

I rewrote this test a bit to rely on warnings.catch_warnings to reset the warnings filters.

With Python 3.14 free-threading, warning.simplefilter does not modify warnings.filters any more inside a warnings.catch_warnings context, see https://docs.python.org/3.14/whatsnew/3.14.html#concurrent-safe-warnings-control.

Copy link
Member

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

Since the remaining test_filter_warning_propagates failure is caused by a bug in joblib/loky, I would just mark it as thread unsafe with an XXX comment linking to the upstream issue.

@lesteve lesteve marked this pull request as ready for review September 2, 2025 05:16
@lesteve
Copy link
Member Author

lesteve commented Sep 2, 2025

codecov being red is likely because there is no coverage in the free-threaded CI build. I guess we could look at this in a further PR but I don't think this is very high priority:
image

@ogrisel ogrisel merged commit ed0a98a into scikit-learn:main Sep 2, 2025
36 of 37 checks passed
@ogrisel
Copy link
Member

ogrisel commented Sep 2, 2025

Thanks @lesteve!

@lesteve lesteve deleted the free-threaded-pytest-run-parallel branch September 2, 2025 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants