✔️ Linting Passed

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

_{Generated for commit: 8b8648d. Link to the linter CI: here}

Whoops bad merge I think! Thanks @jeremiedbb !

Like @jeremiedbb, I would have expected it to be caught by the CI, I guess adding --nitpicky (same as -n) to warn about reference issues would be a first step but not super useful in itself.

Whether it is a good idea to turn this warning into an error is left as an exercise:

• -W i.e. --fail-on-error seems way too strict. For example right now it fails because an extension has not defined its thread-safety

  Warning, treated as error:
  the sphinxcontrib.sass extension does not declare if it is safe for parallel reading, assuming it isn't
  

• grepping the log file for some messages seems doable but would add even more logic to the already involved build_doc.sh script ...

The warnings looks like this:

/home/lesteve/dev/scikit-learn/sklearn/inspection/_permutation_importance.py:docstring of sklearn.inspection._permutation_importance.permutation_importance:32: WARNING: undefined label: 'scoring_nonexistent'

Looking at build_doc.sh if there is a warning in files changed in the PR, CI should fail

So maybe we didn't pick up the error in #30316 because that file did not change (I accepted the merge change from main).

I think this is really an edge case but a potential fix could be to regularly (weekly?) run doc build on main and check for warnings (filtering some out). (i.e. a weekly CI job, not related to PRs)

Note I did build the docs locally a little while ago and noticed some reference (?) warnings, which was on my to-do list to fix a little while ago but I never got round to it 😅

For proof, in #30319 I did a bad merge and there was an undefined label, which did cause CI failure:

https://app.circleci.com/pipelines/github/scikit-learn/scikit-learn/63923/workflows/9340d0d9-da2b-48e7-aa9d-a1e6e4e45c67/jobs/293464

(Note that if you search for 'warning' in the output, there are many other warnings )

This was fixed in commit d25d36b , where you can see CIs passed (i.e. the other warnings did not cause the CI failure, it was just the undefined label).

Indeed, warnings should be turned into errors in

So maybe we didn't pick up the error in #30316 because that file did not change (I accepted the merge change from main).

Ah yeah good point, the issue happened in an unchanged file and build_doc.sh is only checking warnings that are in modified files.

Not entirely sure whether it's worth doing something about it to be honest ...

Agreed, happy to leave it