Add autosummary_filename_map config to avoid clashes by jnothman · Pull Request #7927 · sphinx-doc/sphinx

jnothman · 2020-07-08T06:49:25Z

Subject: Add autosummary_filename_map config to allow users to configure alternative filenames for autodoc objects, avoiding name clashes on case-insensitive file systems.

Feature or Bugfix

Feature
Bugfix

Purpose

This allows the user to configure the mapping between object name and filename when performing autodoc generation of HTML files. It adds a config param called autosummary_filename_map to do so. It is a simple dict between name and filename to be generated.
The intention is that this will allow renames on case-insensitive file systems (per Autodoc may overwrite where functions and classes with different case exist #2024), albeit requiring users to manually identify filenames that might result in name clashes, and propose an alternative filename for the generated docs.

Relates

jnothman · 2020-07-08T06:52:21Z

[fixed branching and force-pushed.]

tk0miya

LGTM with nits! I'll merge this after your update soon :-)

tests/roots/test-autosummary/sphinx.rst

tests/roots/test-ext-autosummary-filename-map/index.rst

tests/test_ext_autosummary.py

tests/roots/test-ext-autosummary-filename-map/conf.py

tests/roots/test-ext-autosummary-filename-map/autosummary_dummy_module.py

sphinx/ext/autosummary/generate.py

jnothman · 2020-07-10T01:30:30Z

Thank you for the review. I hope I have addressed all comments.

tk0miya · 2020-07-10T15:17:10Z

sphinx/ext/autosummary/generate.py

+        filename_map = app.config.autosummary_filename_map
+        if not isinstance(filename_map, Mapping):
+            raise TypeError('autosummary_filename_map should be a mapping from '
+                            'strings to strings')


You don't need to check the value is dict here. The Sphinx automatically checks the given value is the same type as the default value. But it does not check the contents of the dict. I think the default check is enough.

Note: If you'd like to add more careful validations, please use a hook for the config-inited event to validate user configurations.

sphinx/sphinx/builders/latex/__init__.py

Lines 498 to 503 in 98a854d

def validate_latex_theme_options(app: Sphinx, config: Config) -> None:

for key in list(config.latex_theme_options):

if key not in Theme.UPDATABLE_KEYS:

msg = __("Unknown theme option: latex_theme_options[%r], ignored.")

logger.warning(msg % (key,))

config.latex_theme_options.pop(key)

sphinx/sphinx/builders/latex/__init__.py

Line 558 in 98a854d

app.connect('config-inited', validate_latex_theme_options, priority=800)

tk0miya · 2020-07-10T15:19:45Z

sphinx/ext/autosummary/__init__.py

    app.add_config_value('autosummary_mock_imports',
                         lambda config: config.autodoc_mock_imports, 'env')
    app.add_config_value('autosummary_imported_members', [], False, [bool])
+    app.add_config_value('autosummary_filename_map', {}, 'html')


It is better to add a new config variable alphabetically. Could you move this above?
(I noticed autosummary_imported_members is not. I'll fix it later)

thomasjpfan · 2020-07-13T02:11:14Z

Tried this for scikit-learn and got an expected warning:

checking consistency... /Users/thomasfan/Repos/scikit-learn-1/doc/modules/generated/dbscan-lowercase.rst: 
WARNING: document isn't included in any toctree
done

tk0miya · 2020-07-13T18:24:32Z

Oops. Autosummary.run() is also fixed using the new configuration. It constructs a toctree node as a reference to the document file.

jnothman · 2020-07-13T21:47:34Z

I don't really understand... Is there something I should change?

tk0miya · 2020-07-14T14:34:58Z

I think some fix is needed to here. Is my understanding incorrect?

sphinx/sphinx/ext/autosummary/__init__.py

Lines 255 to 257 in 03e1070

    
           for name, sig, summary, real_name in items: 
        
               docname = posixpath.join(tree_prefix, real_name) 
        
               docname = posixpath.normpath(posixpath.join(dirname, docname))

jnothman · 2020-07-15T08:05:53Z

I think some fix is needed to here. Is my understanding incorrect?

Seems reasonable. I suppose I need to come up with a test case.

jnothman · 2020-07-21T05:11:19Z

This is ready for review.

tk0miya

LGTM with nits. I'll merge this after the fix soon.

tests/roots/test-ext-autosummary-filename-map/index.rst

tk0miya · 2020-07-23T16:07:29Z

Just merged. Thank you for your contribution!

Add autosummary_filename_map config to avoid clashes

72ca2bd

jnothman force-pushed the name-case-clash branch from 6df159d to 72ca2bd Compare July 8, 2020 06:52

jnothman added 2 commits July 8, 2020 20:29

Flake8

d401054

Fix attempt with new test root

4e0ff5c

tk0miya requested changes Jul 9, 2020

View reviewed changes

tk0miya added extensions:autosummary type:proposal a feature suggestion labels Jul 9, 2020

tk0miya added this to the 3.2.0 milestone Jul 9, 2020

Responding to comments

52c173b

Simplify dummy module

71d1b37

tk0miya reviewed Jul 10, 2020

View reviewed changes

jnothman added 2 commits July 15, 2020 18:46

Fix toctree generation

aae8ce8

Rm unused import

2b843dd

tk0miya requested changes Jul 23, 2020

View reviewed changes

tests/roots/test-ext-autosummary-filename-map/index.rst Outdated Show resolved Hide resolved

Remove irrelevant test RST

144f18a

tk0miya approved these changes Jul 23, 2020

View reviewed changes

tk0miya merged commit da17413 into sphinx-doc:3.x Jul 23, 2020

tk0miya added a commit that referenced this pull request Jul 23, 2020

Update CHANGES for PR #7927

3d0818f

github-actions bot locked as resolved and limited conversation to collaborators Jul 25, 2021

	def validate_latex_theme_options(app: Sphinx, config: Config) -> None:
	for key in list(config.latex_theme_options):
	if key not in Theme.UPDATABLE_KEYS:
	msg = __("Unknown theme option: latex_theme_options[%r], ignored.")
	logger.warning(msg % (key,))
	config.latex_theme_options.pop(key)

Uh oh!

Conversation

jnothman commented Jul 8, 2020

Feature or Bugfix

Purpose

Relates

Uh oh!

jnothman commented Jul 8, 2020

Uh oh!

tk0miya left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jnothman commented Jul 10, 2020

Uh oh!

tk0miya Jul 10, 2020

Choose a reason for hiding this comment

Uh oh!

tk0miya Jul 10, 2020

Choose a reason for hiding this comment

Uh oh!

thomasjpfan commented Jul 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tk0miya commented Jul 13, 2020

Uh oh!

jnothman commented Jul 13, 2020 via email

Uh oh!

tk0miya commented Jul 14, 2020

Uh oh!

jnothman commented Jul 15, 2020

Uh oh!

jnothman commented Jul 21, 2020

Uh oh!

tk0miya left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tk0miya commented Jul 23, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

thomasjpfan commented Jul 13, 2020 •

edited

Loading