-
Notifications
You must be signed in to change notification settings - Fork 1k
feat(scripts/autolabel): allow multiple labels, avoid labels of dependent topic areas #34066
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
base: master
Are you sure you want to change the base?
feat(scripts/autolabel): allow multiple labels, avoid labels of dependent topic areas #34066
Conversation
PR summary ca7d6f198fImport changes for modified filesNo significant changes to the import graph Import changes for all files
|
8e5fb0a to
062eb43
Compare
062eb43 to
52c0914
Compare
b6aca94 to
e549fe7
Compare
Co-authored-by: damiano <[email protected]>
| * The `label` field is the actual GitHub label name. | ||
| Array of all topic labels which are used in Mathlib. | ||
| Note: Since Lean does not know reflection, this needs to be updated manually! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about we add a test that at least notices if the lists are not in sync? What is below works and assumes that the two lists are exactly equal (not just up to permutations, although we could factor that in as well).
open Lean in
run_cmd
let some (.inductInfo labels) := (← getEnv).find? ``Label | throwError "Unreachable"
let labelNames := labels.ctors.filterMap (·.components.getLast?) |>.toArray
let some (.defnInfo dinfo) := (← getEnv).find? ``mathlibLabels | throwError "Unreachable"
let allConstants := dinfo.value.getUsedConstants
let constants := allConstants.filterMap fun n =>
if n.components.dropLast.getLast? == some `Label then n.components.getLast? else none
if labelNames != constants then
logWarning m!"The available Labels is out of sync with the labels listed in \
{.ofConstName ``mathlibLabels}.\n\
Please keep them in sync!"
Refactor autolabelling script to include two features:
1)This PR does NOT change the behaviour of the autolabel bot, it merely implements the functionality to do so later.
The proposed dependencies will be added as follow-ups, tracked by #34078.
Testing
Modify some files somewhere in Mathlib, make a temporary commit with these changes and run
lake exe autolabelThis should print all labels which would be added to the PR.Note, that
autolabelcompares the current HEAD toorigin/master(notmasterorupstream/master!) so make sure theorigin/masterof your fork is up-to-date.This PR is split into 2 commits, which can be reviewed separately.
t-algebraic-geometryis added,t-ring-theoryshould never be added". But do not provide any values for this featureZulip discussion: #mathlib reviewers > Proposal: automatic reviewer assignment @ 💬