MAINT: Work around Sphinx deprecation

[larsoner]

Not 100% sure if this is a correct workaround for this deprecation:

...
  File "/home/larsoner/python/pydata-sphinx-theme/pydata_sphinx_theme/bootstrap_html_translator.py", line 27, in visit_table
    self.generate_targets_for_table(node)
  File "/home/larsoner/python/sphinx/sphinx/writers/html5.py", line 780, in generate_targets_for_table
    warnings.warn('generate_targets_for_table() is deprecated',
sphinx.deprecation.RemovedInSphinx60Warning: generate_targets_for_table() is deprecated

But it seems to work locally at least.

[jorisvandenbossche]

Thanks for looking into this

I wonder if we could remove it entirely, since the sphinx visit_table also doesn't call it anymore (https://github.com/sphinx-doc/sphinx/blob/424510e326f3fbdc30f34dcd2865e38edb38aa2d/sphinx/writers/html5.py#L716-L729), and the docstring of generate_targets_for_table seems to indicate it was for a bug in an older version of docutils.

[jorisvandenbossche]

It indeed seems to be deprecated in a PR (sphinx-doc/sphinx#7264) that removed support for docutils 0.13 and 0.14

[larsoner]

It looks like that will be true for Sphinx 4, but I'm not sure when that will actually land (they say Spring at least) and when it will be adopted by people. It looks like it is still called in the 3.x maintenance branch. Given the tentativeness of the release date and how slow people will be to upgrade sphinx once 4 comes out, seems worthwhile to me to keep some code in for backward compat.

I could add something instead like if LooseVersion(sphinx.__version__) >= LooseVersion('4.0'): return super().visit_table(node) which would both avoid the deprecation warning and make it clear that the override can go away entirely once only version 4 is supported, WDYT?

[jorisvandenbossche]

With "removing entirely", I mean only the self.generate_targets_for_table(node) line, and not the full visit_table (which we still need I think to change some classes).
But so putting self.generate_targets_for_table(node) behind a sphinx version check sounds good

[larsoner]

But so putting self.generate_targets_for_table(node) behind a sphinx version check sounds good

Done. I used from distutils.version import LooseVersion but apparently distutils itself is deprecated. Apparently the alternative is to use packaging.version.parse instead, which I could do but I guess this would then have to live in requires somewhere in setup.py. It's not too onerous since every Python install probably has setuptools anyway but for now maybe LooseVersion is good enough until things actually need to be updated.

[jorisvandenbossche]

Yeah, the distutils deprecation is something we are going to handle in many places (many projects use distutils.version in runtime, I think), so that can be dealt with later.

[jorisvandenbossche]

Thanks!