Skip to content

ENH: Allow plain list as subsection_order and support a wildcard#1295

Merged
larsoner merged 3 commits intosphinx-gallery:masterfrom
timhoffm:explicit-order
Apr 26, 2024
Merged

ENH: Allow plain list as subsection_order and support a wildcard#1295
larsoner merged 3 commits intosphinx-gallery:masterfrom
timhoffm:explicit-order

Conversation

@timhoffm
Copy link
Copy Markdown
Contributor

Closes #1293. Should wait for #1289.

Note: For simplicity, I've modified ExplicitOrder to include the wildcard. This is backward-compatible. One could alternatively make an independent internal ListOrder sortkey if ExplicitOrder should not be touched.

Note 2: Elements in the ordered_list that does not exists as a subsection are silently ignored. This is present behavior and we cannot detect this as long as the pattern sorted(..., key=) with a key function. If we want to warn on that as well (I've proposed that in in #1293), we would need to write a custom code path for sorting if 'subsection_order' is a list. For simplicity, I've not done that. It's always possible to do this later.

Copy link
Copy Markdown
Contributor

@lucyleeow lucyleeow left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for adding tests!

Comment on lines +385 to +478
Even more generally, you can set 'subsection_order' to any callable, which
will be used as sorting key function on the subsection paths. In fact, the
above list is a convenience shortcut and it is internally wrapped in
:class:`sphinx_gallery.sorting.ExplicitOrder` as a sortkey.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Lets move this down, below we talk about using your own sort key callable and link to 'own_sort_keys' below (line 410). Or at least join these 2 sections together

Copy link
Copy Markdown
Contributor Author

@timhoffm timhoffm Apr 24, 2024

Choose a reason for hiding this comment

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

I'm not following. The current story line is

  1. default: alphabetical order
  2. use a list to define a custom order, optionally with a wildcard
  3. more generally, you can use a sort key function
  4. passing a list is only a shortcut for ExplicitOrder sortkey; using the list is recommended now.
  5. technical details on sort keys.

You recommend to move 3 and 4 down? I believe 3 cannot be moved down. We first have to introduce the sortkey concept. One could move 4 down. However, my motivation to put it here was that "list replaces ExplicitOrder" is something that is relevant for many existing users and should not be moved too far down. The technical details on sort keys are only relevant if write custom sort keys.

What exactly do you suggest? Alternatively, feel free to push the desired change. That may be simpler than trying to tell me. 😄

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Join 3 and 5. I read 5 as intro to using own sort keys.

Also do you introduce wildcards?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh I meant the last paragraph, not '5', I think. This one:

If you implement your own sort key, it will be passed the subfolder path,
relative to the ``conf.py`` file. See :ref:`own_sort_keys` for more information.

Add this to 3

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. (I've included the ref above. The rest ist already mentioned there.)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yup perfect. But we don't mention use of wildcard?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Where are you missing it? We have it

  • in the configuration docs (configuration.rst l.469)
  • in the ExplicitOrder docstring (sorting.py l.23)
  • in the error message for missing paths (sorting.py l.82)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry missed it. Looks good

@timhoffm timhoffm force-pushed the explicit-order branch 5 times, most recently from 1fc84f1 to 4a78a6f Compare April 24, 2024 20:26
This is equivalent to the list wrapped in ExplicitOrder
Copy link
Copy Markdown
Contributor

@lucyleeow lucyleeow left a comment

Choose a reason for hiding this comment

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

Thanks!

@larsoner do you have time to release?

@larsoner larsoner merged commit 2120054 into sphinx-gallery:master Apr 26, 2024
@larsoner
Copy link
Copy Markdown
Contributor

Thanks @timhoffm !

@lucyleeow I could do it probably Tues/Wed next week. Feel free to cut a release today or Monday if you have time!

@timhoffm timhoffm deleted the explicit-order branch April 26, 2024 19:00
clrpackages referenced this pull request in clearlinux-pkgs/pypi-sphinx_gallery Apr 30, 2024
… to version 0.16.0

v0.16.0
-------
Sphinx 7.3.0 and above changed caching and serialization checks. Now instead of passing
instantiated classes like ``ResetArgv()``, classes like ``FileNameSortKey``, or
callables like ``notebook_modification_function`` in  ``sphinx_gallery_conf``,
you should pass fully qualified name strings to classes or callables. If you change
to using name strings, you can simply use a function as the use of classes to ensure
a stable ``__repr__`` would be redundant.

See :ref:`importing_callables` for details.

**Implemented enhancements:**

-  ENH: Allow plain list as subsection_order and support a wildcard `#1295 <https://github.com/sphinx-gallery/sphinx-gallery/pull/1295>`__ (`timhoffm <https://github.com/timhoffm>`__)
-  [ENH] Minigallery can take arbitrary files/glob patterns as input `#1226 <https://github.com/sphinx-gallery/sphinx-gallery/pull/1226>`__ (`story645 <https://github.com/story645>`__)

(NEWS truncated at 15 lines)
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.

ENH: Allow a plain list as subsection_order configuration

3 participants