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

Add rtd-sphinx-search #2805

Merged
merged 7 commits into from
Jan 15, 2024
Merged

Add rtd-sphinx-search #2805

merged 7 commits into from
Jan 15, 2024

Conversation

flying-sheep
Copy link
Member

@flying-sheep flying-sheep commented Jan 11, 2024

  • Release notes not necessary because:

Adds readthedocs-sphinx-search support via the scanpydoc theme, which contains JS and CSS customizations to make the search extension integrate with the theme. See

rendered

An alternative that looks nicer would be https://github.com/readthedocs/addons, but it’s still in alpha

PS: I didn’t add the same hack as in scanpydoc that makes the search work in PR builds, so you’ll only see “No results found” in the above. Check out https://icb-scanpydoc.readthedocs-hosted.com/en/latest/?rtd_search=scanpydoc to see rendered search results. You can see that the API works for scanpy:

$ http get https://icb-scanpy.readthedocs-hosted.com/_/api/v3/search/?q=project%3Aicb-scanpy%2Flatest+filter_cells
╭──────────┬────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│ count    │ 2                                                                                                                                                                                                                                                                                  │
│ next     │                                                                                                                                                                                                                                                                                    │
│ previous │                                                                                                                                                                                                                                                                                    │
│          │ ╭───┬────────────┬────────────────╮                                                                                                                                                                                                                                                │
│ projects │ │ # │    slug    │    versions    │                                                                                                                                                                                                                                                │
│          │ ├───┼────────────┼────────────────┤                                                                                                                                                                                                                                                │
│          │ │ 0 │ icb-scanpy │ ╭───┬────────╮ │                                                                                                                                                                                                                                                │
│          │ │   │            │ │ # │  slug  │ │                                                                                                                                                                                                                                                │
│          │ │   │            │ ├───┼────────┤ │                                                                                                                                                                                                                                                │
│          │ │   │            │ │ 0 │ latest │ │                                                                                                                                                                                                                                                │
│          │ │   │            │ ╰───┴────────╯ │                                                                                                                                                                                                                                                │
│          │ ╰───┴────────────┴────────────────╯                                                                                                                                                                                                                                                │
│ query    │ filter_cells                                                                                                                                                                                                                                                                       │
│          │ ╭───┬──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┬─────╮ │
│ results  │ │ # │                                                                                                                                blocks                                                                                                                                │ ... │ │
│          │ ├───┼──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼─────┤ │
│          │ │ 0 │ ╭───┬────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┬─────╮ │ ... │ │
│          │ │   │ │ # │                                                                                                                        content                                                                                                                         │ ... │ │     │ │
│          │ │   │ ├───┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼─────┤ │     │ │
│          │ │   │ │ 0 │ figdir (scanpy._settings.ScanpyConfig property) file_format_data (scanpy._settings.ScanpyConfig property) file_format_figs (scanpy._settings.ScanpyConfig property) filter_cells() (in module scanpy.pp) filter_genes() (in module scanpy.pp)          │ ... │ │     │ │
│          │ │   │ │   │ filter_genes_dispersion() (in module scanpy.pl) (in module scanpy.pp) filter_rank_genes_groups() (in module scanpy.tl)                                                                                                                                 │     │ │     │ │
│          │ │   │ ╰───┴────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┴─────╯ │     │ │
│          │ │ 1 │ ╭───┬────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┬─────╮ │ ... │ │
│          │ │   │ │ # │                                                                                                                        content                                                                                                                         │ ... │ │     │ │
│          │ │   │ ├───┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼─────┤ │     │ │
│          │ │   │ │ 0 │ Normalize total counts per cell. Warning Deprecated since version 1.3.7: Use normalize_total() instead. The new function is equivalent to the present function, except that the new function doesn’t filter cells based on min_counts, use             │ ... │ │     │ │
│          │ │   │ │   │ filter_cells() if filtering is needed. some arguments were renamed copy is replaced by inplace Normalize each cell by total counts over all genes, so that every cell has the same total count after normalization. Similar functions are used, for    │     │ │     │ │
│          │ │   │ │   │ example, by Seurat [Satija15], Cell Ranger [Zheng17] or SPRING [Weinreb17]. Parameters: data AnnData | ndarray | spmatrixThe (annotated) data matrix of shape n_obs × n_vars. Rows correspond to cells and columns to genes. counts_per_cell_after     │     │ │     │ │
│          │ │   │ │   │ Optional[float] (default: None)If None, after normalization, each cell has a total count equal to the median of the counts_per_cell before normalization. counts_per_cell Optional[ndarray] (default: None)Precomputed counts per cell. key_n_counts   │     │ │     │ │
│          │ │   │ │   │ str (default: 'n_counts')Name of the field in adata.obs where the total counts per cell are stored. copy bool (default: False)If an AnnData is passed, determines whether a copy is returned. min_counts int (default: 1)Cells with counts less than   │     │ │     │ │
│          │ │   │ │   │ min_counts are filtered out during normalization. Return type: UnionType[AnnData, ndarray, spmatrix, None] Returns: Returns None if copy=False, else returns an updated AnnData object. Sets the following fields: adata.Xnumpy.ndarray |              │     │ │     │ │
│          │ │   │ │   │ scipy.sparse._csr.csr_matrix (dtype float)Normalized count data matrix. Examples >>> import scanpy as sc >>> adata = AnnData(np.array([[1, 0], [3, 0], [5, 6]], dtype=np.float32)) >>> print(adata.X.sum(axis=1)) [ 1. 3. 11.] >>>                     │     │ │     │ │
│          │ │   │ │   │ sc.pp.normalize_per_cell(adata) >>> print(adata.obs) n_counts 0 1.0 1 3.0 2 11.0 >>> print(adata.X.sum(axis=1)) [3. 3. 3.] >>> sc.pp.normalize_per_cell( ... adata, counts_per_cell_after=1, ... key_n_counts='n_counts2', ... ) >>> print(adata.obs)  │     │ │     │ │
│          │ │   │ │   │ n_counts n_counts2 0 1.0 3.0 1 3.0 3.0 2 11.0 3.0 >>> print(adata.X.sum(axis=1)) [1. 1. 1.]                                                                                                                                                            │     │ │     │ │
│          │ │   │ ╰───┴────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┴─────╯ │     │ │
│          │ ╰───┴──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┴─────╯ │
╰──────────┴────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯

Copy link

codecov bot commented Jan 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c410cd1) 72.84% compared to head (db26f06) 72.70%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2805      +/-   ##
==========================================
- Coverage   72.84%   72.70%   -0.14%     
==========================================
  Files         111      111              
  Lines       12368    12368              
==========================================
- Hits         9009     8992      -17     
- Misses       3359     3376      +17     

see 1 file with indirect coverage changes

@flying-sheep flying-sheep marked this pull request as draft January 11, 2024 14:09
@flying-sheep flying-sheep self-assigned this Jan 12, 2024
@flying-sheep flying-sheep marked this pull request as ready for review January 12, 2024 09:40
@flying-sheep flying-sheep requested a review from ivirshup January 12, 2024 09:40
Copy link
Member

@ivirshup ivirshup left a comment

Choose a reason for hiding this comment

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

I'm running into some bugs related to the theme's search:

First, this shortcut is shown though I think / is the shortcut used by sphinx.

image

If I hit Cmd-K I get:

image

If I Esc out of that I get:

image

Is it possible to just not ship the javascript/ css that the pydata theme uses for search?


Code changes otherwise look good.

@flying-sheep
Copy link
Member Author

flying-sheep commented Jan 12, 2024

Is it possible to just not ship the javascript/ css that the pydata theme uses for search?

No, they have all their JS in one file which we don’t want to re-implement. But I’m pretty sure I know how to fix this issue.

@flying-sheep
Copy link
Member Author

flying-sheep commented Jan 12, 2024

OK, done! theislab/scanpydoc#128 is released and this PR is updated.

I’m not touching the / shortcut: meta/ctrl+k is advertised in a prominent spot and I don’t want to accidentally swallow a user trying to type / somewhere.

@flying-sheep flying-sheep requested a review from ivirshup January 12, 2024 14:10
Copy link
Member

@ivirshup ivirshup left a comment

Choose a reason for hiding this comment

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

Found a couple more things. Mainly shortcut related:

Shortcut overriding system shortcuts

Typically Cmd-Delete deletes a whole line of text for me. This doesn't happen in the search bar for the pydata sphinx theme. However it does work on the readthedocs site.

Can we prevent the theme from intercepting and overriding this hotkey?

Shortcut opens but does not close search palette

Cmd-K will open, but not close the search palette. On the pydata sphinx demo site it both opens and closes the palette.

On readthedocs, / will only open the palette and Esc is needed to close it (AFAICT).


For both of these, I think it would be nice to just be a bit more consistent with one of the options.

Are any improvements here reasonably easy to do? I recognize that it's making two libraries talk to each other, and at least one of them can't be totally turned off, so this might be difficult.

"Hide search matches" button appearing when it shouldn't (no action needed)

This is unrelated to the PR and seems to happen for all our PR builds. No action needed, just noting it

If I search for something, hit enter to go to the full results, then go back to the main page I get this little banner on top:

image

Note that this doesn't happen just for search results. It happens for any link I click, eg: table of contents. This doesn't happen on our live sites though.

@flying-sheep
Copy link
Member Author

Are any improvements here reasonably easy to do? I recognize that it's making two libraries talk to each other, and at least one of them can't be totally turned off, so this might be difficult.

yes, both things are quite easy, I think. Event listeners can be registered in a way that they don’t capture the event, and toggling the popup is a simple if (isModalVisible()) removeSearchModal() else showSearchModal().

@flying-sheep
Copy link
Member Author

flying-sheep commented Jan 15, 2024

All done! The “eats my ctrl/+” problem isn’t from my customizations, it’s pydata/pydata-sphinx-theme#1645

@flying-sheep flying-sheep requested a review from ivirshup January 15, 2024 10:34
@ivirshup ivirshup merged commit 30c5902 into master Jan 15, 2024
13 checks passed
@ivirshup ivirshup deleted the docs-search branch January 15, 2024 14:41
@flying-sheep flying-sheep linked an issue Jan 25, 2024 that may be closed by this pull request
3 tasks
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.

Broken search results on the website
2 participants