[MRG] Patches sphinx.ext.autosummary for case insensitive file systems

[thomasjpfan]

Reference Issues/PRs

This is an alternative solution to issue #12712. This PR or PR #12968 can address the issue.

What does this implement/fix? Explain your changes.

This PR adds configuration options to change the suffix:

 custom_autosummary_names_with_new_suffix = {
    'sklearn.cluster.dbscan',
    'sklearn.cluster.optics',
    'sklearn.covariance.oas',
    'sklearn.decomposition.fastica'
}
custom_autosummary_new_suffix = '-lowercase.rst'
custom_autosummary_generated_dirname = os.path.join('modules', 'generated')

This PR monkeypatches os.path.join used by sphinx.ext.autosummary.generate to return a filename with a new suffix.

Any other comments?

This PR contains less logic and gives the control to the user, thus a little simpler when compared to #12968.

[jnothman]

The difference being that the other pr automatically identifies collisions? Yes, this is simpler to read

[thomasjpfan]

Yes the other PR is automatic, while this one requires a user to add a function to custom_autosummary_names_with_new_suffix if there is a conflict. Since both solutions monkey patches, I think a simpler to read version is better.

[thomasjpfan]

There is an issue in sphinx regarding this issue: sphinx-doc/sphinx#1495

[rth]

It's a nice fix, but honestly I do wonder if we want to maintain this. The only affected users are maintainers on Mac OS as far as I can tell, and the solution is not that trivial. Proposing it in sphinx would be ideal.

[thomasjpfan]

Yea, I wish this was less hacky. The logic of "check if file exist by not using the file system and renaming the file to something" is kind of weird to configure (from a sphinx user point of view)

[jnothman]

This is ancient, but if it's not been fixed upstream in Sphinx (we should check), we should consider merging it before trying to create a zipped-HTML release of the docs.

[thomasjpfan]

We are still using this for update the docs for dash-docs.

Last time I looked at this, autosummary was using the filenames as the "source of truth" and this only works for case sensitive file systems.

From the original issue: sphinx-doc/sphinx#1495 it looks like this issue has been pushed up to "Some future version" milestone. (Sounds familiar eh?)

[thomasjpfan]

I have a feeling this would introduce new sphinx warnings and fail the CI (now that it catches sphinx warnings).

Edit - Oh wow it worked

[adrinjalali]

Let's merge this one so that we can proceed on #17051?

[jnothman]

This has broken the doc build:

Traceback (most recent call last):
  File "/home/circleci/miniconda/envs/testenv/lib/python3.8/site-packages/sphinx/cmd/build.py", line 276, in build_main
    app = Sphinx(args.sourcedir, args.confdir, args.outputdir,
  File "/home/circleci/miniconda/envs/testenv/lib/python3.8/site-packages/sphinx/application.py", line 244, in __init__
    self.setup_extension(extension)
  File "/home/circleci/miniconda/envs/testenv/lib/python3.8/site-packages/sphinx/application.py", line 398, in setup_extension
    self.registry.load_extension(self, extname)
  File "/home/circleci/miniconda/envs/testenv/lib/python3.8/site-packages/sphinx/registry.py", line 414, in load_extension
    metadata = setup(app)
  File "/home/circleci/project/doc/sphinxext/custom_autosummary_new_suffix.py", line 76, in setup
    for listener_id, obj in builder_inited_listeners.items():
AttributeError: 'list' object has no attribute 'items'

[adrinjalali]

should I revert it or are you gonna do it?

[bsipocz]

@thomasjpfan - I was so happy to see you have a solution for this problem here, and sad to see it needed to be reverted.
Did you come up with something else instead?

[thomasjpfan]

@bsipocz The fix was added to sphinx directly in sphinx-doc/sphinx#7927 with the autosummary_filename_map feature. Scikit-learn makes uses the feature as follows:

scikit-learn/doc/conf.py

Lines 583 to 589 in 4db0492

	# maps functions with a class name that is indistinguishable when case is
	# ignore to another filename
	autosummary_filename_map = {
	"sklearn.cluster.dbscan": "dbscan-function",
	"sklearn.covariance.oas": "oas-function",
	"sklearn.decomposition.fastica": "fastica-function",
	}