Repository navigation
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
skirpichev
left a comment
There was a problem hiding this comment.
I'm played with this a bit and I generally like new search output 👍
...to try to keep most search results roughly where they currently are.
|
Thanks for taking a look, @skirpichev! I just pushed a few changes, adding some comments and tweaking the scoring a little bit (to adjust one value that I inadvertently changed from the sphinx defaults that was affecting the ordering more than I intended). Looking around a bit more, though, I am wondering if it might be better to do this with a Sphinx object domain, though, setting the builtin things to be important. But I'm not sure if there's an easy way to do that without hard-coding all of the built-in objects somewhere. @AA-Turner might know better. Then again, some of the other changes I had been thinking of probably do require a |
skirpichev
left a comment
There was a problem hiding this comment.
Look good for me, modulo Stan's comment.
I am wondering if it might be better to do this with a Sphinx object domain, though, setting the builtin things to be important
Probably, the right way might be a mixture of both.
|
Thanks! I went ahead and moved the file into |
|
This PR is stale because it has been open for 30 days with no activity. |
| let [docname, title, anchor, descr, score, filename] = result; | ||
|
|
||
| // boost the score of built-in functions and types | ||
| const builtinPages = ["library/stdtypes", "library/functions"]; |
There was a problem hiding this comment.
Let's add library/exceptions and library/constants.
There was a problem hiding this comment.
Thanks for taking a look, and good call! I just added those and pushed.
Documentation build overview
128 files changed ·
|
|
|
||
| return score; | ||
| }, | ||
|
|
There was a problem hiding this comment.
Please add a comment clarifying these are the defaults, as per the docs.
There was a problem hiding this comment.
Sorry to be a while getting back to this. I did used to have a comment in there saying exactly this but removed it after an earlier code review comment (in 7e21628). Should I put it back?
There was a problem hiding this comment.
Sounds good. I just added that comment back (along with a reference to the Sphinx docs).
| '<meta property="og:image:height" content="200">', | ||
| ) | ||
|
|
||
| # Options to adjust search results sorting |
There was a problem hiding this comment.
Please move this up, above configuration for the various extensions, beside the Sphinx adjustments.
|
I'm not sure what was causing the CI failure before, but merging from |
| @@ -0,0 +1,40 @@ | |||
| var Scorer = { | |||
There was a problem hiding this comment.
Can we use const or let instead of var?
There was a problem hiding this comment.
We actually need var here because of some stuff inside of Sphinx. I just tried again (both const and let) and was getting errors from Sphinx's searchtools.js, which conditionally defines Scorer if it isn't already defined (see here).
| let [docname, title, anchor, descr, score, filename] = result; | ||
|
|
||
| // boost the score of built-in functions and types | ||
| const builtinPages = [ |
There was a problem hiding this comment.
Following #156682, builtins have been split out. Update to include?
Also alphabetise these.
There was a problem hiding this comment.
Thanks; just updated now (this patch didn't really do much without pointing to the updated page locations!).
| "library/constants", | ||
| ]; | ||
| if (builtinPages.includes(docname)) { | ||
| score += 7; |
There was a problem hiding this comment.
It wasn't a terribly scientific process. IIRC, I tried 5 and it didn't push int or set to the top of their search pages, so I tried a couple of different values until I was generally happy with where the built-ins landed.
|
Not sure why the readthedocs build is failing (it looks like it somehow couldn't find that most recent commit?). Is there a way to force it to try again? |
This patch adds a custom search scorer for the Python docs, in the hopes of making the search results more useful. For now, this just increases the score of built-in functions and types so that they show up closer to the top of the relevant search results, though it also opens the door to other improvements. I also have a couple of other ideas that might further improve the ordering, but I figured it'd be best to start small.
With this change:
strtype goes from being the 44th result to the 1st.settype goes from being the 14th result to the 1st.inttype now shows up abovetkinterandpprint.I don't think this needs a news item, but I can add one if y'all think it's necessary.
📚 Documentation preview 📚: https://cpython-previews--138278.org.readthedocs.build/