Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use sphinx builtin only directive instead of custom one. #11295

Merged
merged 1 commit into from
May 25, 2018

Conversation

timhoffm
Copy link
Member

PR Summary

Switch from our custom .. htmlonly:: and .. latexonly:: directives to the sphinx builtin .. only:: html and .. only:: latex.

AFAICS they do exactly the same. So let's get rid of some proprietary stuff and just use plain sphinx.
Our directives are from mid 2008. Sphinx itself introduced that functionally just slightly later in Release 0.6 (Mar 24, 2009).

@anntzer anntzer added this to the v3.0 milestone May 23, 2018
Copy link
Member

@dstansby dstansby left a comment

Choose a reason for hiding this comment

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

Should we deprecate only_directives.py instead of just deleting it?

@timhoffm
Copy link
Member Author

timhoffm commented May 24, 2018

If we want to be very strict in our deprecation policy, yes.

However, the only use case for these would be in the documentation of downstream projects. Building documentation is happening pre-release, so they will notice and can simply replace the expressions. The only advantage of a deprecation would be that the downstream projects have a bit more time for the change. Given the timing (introduced mid 2008, superseeded early 2009 by sphinx directives), I question if anybody else uses our (undocumented) directives at all.

I would be practical here and dare removing it straight away. But if you feel a deprecation is better, I can add one.

@dstansby
Copy link
Member

Yeah, I think I would be in favour of deprecating just in case, and then removing in 3.1.

@matthew-brett
Copy link
Contributor

The problem with deprecation is that most doc builds are already pretty noisy in their warning output. I bet most people won't notice an extra deprecation warning.

@dstansby
Copy link
Member

People might notice a deprecation changelog entry though? I'm not too bothered either way about deleting/deprecating, but either way this needs a changelog entry.

@matthew-brett
Copy link
Contributor

I don't know about you, but I've never got my first warning of a deprecation from a changelog entry. If I see one, or I get an error, then I might go looking.

@jklymak jklymak merged commit f87c686 into matplotlib:master May 25, 2018
@jklymak
Copy link
Member

jklymak commented May 25, 2018

Merged. If someone wants to get excited about adding a deprecation, thats fine, but I don't think the case is very strong.

@dstansby
Copy link
Member

Can this get a changelog entry?

@tacaswell
Copy link
Member

Downstream users of this included by day-job 😞

tacaswell added a commit to danielballan/ophyd that referenced this pull request Oct 11, 2018
This was removed in Matplotlib 3.0 and we did not actually use it

xref matplotlib/matplotlib#11295
@timhoffm timhoffm deleted the only-directive branch October 11, 2018 18:46
danielballan pushed a commit to danielballan/ophyd that referenced this pull request Oct 11, 2018
This was removed in Matplotlib 3.0 and we did not actually use it

xref matplotlib/matplotlib#11295
danielballan pushed a commit to danielballan/ophyd that referenced this pull request Oct 11, 2018
This was removed in Matplotlib 3.0 and we did not actually use it

xref matplotlib/matplotlib#11295
@bjlittle bjlittle mentioned this pull request Oct 16, 2019
3 tasks
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.

None yet

6 participants