BUG: Fix serialization with Sphinx 7.3

[larsoner]

Closes #1286

1. Support fully qualified names of classes / callables
2. Add aliases for our own ones like "FileNameSortKey" as an alias for "sphinx_gallery.sorting.FileNameSortKey"
3. Keep sphinx_gallery_conf clean: don't store app in there or any functions
   1. Check for existence when filling the conf
   2. Load them on the fly when they need to be used
4. Add to release notes

Kind of a pain to figure out but ultimately it does seem cleaner!

[larsoner]

@maxnoe can you see if the documented changes here make sense to you?

[larsoner]

@lucyleeow ready for review/merge from my end

[lucyleeow]

Thank you for fixing this, does not look like it was a simple fix. Lots of nits that can be fixed later really.

[NirobNabil]

subsection_order field on the conf still requires a callable which is called directly in gen_gallery and thus generates the non pickleable warning. I also went through the tests of sphinx_gallery and couldn't find any testcase covering the subsection_order field making sure having a callable in this field doesn't mess up anything.

while working on the same issue at matplotlib i made some changes in gen_gallery and gen_rst to handle the issue with subsection_order and it seemed to build without any issue.

• changed line 447 in gen_gallery to

  sortkey = _get_class(gallery_conf, "subsection_order")

• and changed the _get_class function in gen_rst at line 1503 to

  if not inspect.isclass(val) and not hasattr(val, '__class__'):
          raise ConfigError(
              f"{what} must be 1) a fully qualified name (string) that resolves "
              f"to a class or 2) or a class itself 3) or an instance of a class,"
              f" got {val.__class__.__name__} ({repr(val)})"
          )

  so that it passes with both a class definition or an instance of a class

[larsoner]

I'll probably try modifying it just to use _get_callables instead so that a callable needs to be passed rather than a class

[larsoner]

Pushed a commit, feel free to give it a try / another look @NirobNabil @lucyleeow !

[lucyleeow]

Not sure what this means.

Maybe we could add a docstring one liner?

[larsoner]

it refers to the following assert line which should always pass because we've ensured it elsewhere

[larsoner]

Okay docs hopefully fixed. Realized there were a few to fix in advanced.rst as well!

[NirobNabil]

Been waiting for this PR to go into a release so that the fix at matplotlib can be merged. Any idea when the next release window of sphinx-gallery might be?

[lucyleeow]

We'll release after this PR goes in.

Edit: 🤦 wait I better investigate CI failures (#1291 (review)) first nevermind, this PR fixes them

[lucyleeow]

Haven't looked deeply in the tests but should we parametrize some to test config value being str and callable (since we still want to support callable atm). Can be done in separate PR. I guess we would eventually remove callable support if Sphinx moves to toml conf file.

Thanks for fixing this, not an easy one.

[larsoner]

@lucyleeow are you up for pushing directly? I think it will be faster

[lucyleeow]

Can be done in separate PR but what do you think about just updating the gallery_conf value in _get_callables, so we don't have to do it twice (call _get_callables twice)?

[lucyleeow]

@lucyleeow are you up for pushing directly? I think it will be faster

Done, only changed docs.

[larsoner]

what do you think about just updating the gallery_conf value in _get_callables, so we don't have to do it twice (call _get_callables twice

I wish we could but the env (including gallery conf) is cached after SG runs so it would cause the same serializing problems

[lucyleeow]

Ah I didn't realise that. Can't see a way around that. Feel free to merge!

[lucyleeow]

Ah I forgot about the tests, I can update in a separate PR.

[larsoner]

Yeah let's get this in and I can start testing with MNE-Python for example. Then maybe release Friday?