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

add a setting to enable / disable the numpy type preprocessor #8095

Merged
merged 4 commits into from Aug 13, 2020

Conversation

keewis
Copy link
Contributor

@keewis keewis commented Aug 10, 2020

Feature or Bugfix

  • Bugfix

Purpose

This adds the napoleon_preprocess_types setting (could also be napoleon_preprocess_numpy_types or napoleon_link_types / napoleon_link_numpy_types) that controls whether the preprocessor is run. Defaults to True for now.

Relates

@keewis keewis changed the title add a setting to enable / display the numpy type preprocessor add a setting to enable / disable the numpy type preprocessor Aug 10, 2020
@keewis
Copy link
Contributor Author

keewis commented Aug 10, 2020

@tk0miya, I think the only issues left are the naming and the default value. I'm not sure about both, but I guess making the preprocessor opt-in would make the migration smoother.

@tk0miya
Copy link
Member

tk0miya commented Aug 12, 2020

I think the only issues left are the naming and the default value. I'm not sure about both, but I guess making the preprocessor opt-in would make the migration smoother.

+1 for the naming. Let's go with this.

About the default value, I'm still debating. It might be better to make disabled it by default if many projects uses types like type-hints. And it will be enabled after we'll support them in the future release. But I don't know how many projects do that. What do you think?

@keewis
Copy link
Contributor Author

keewis commented Aug 12, 2020

I don't know how many projects are using type hints instead of numpydoc type specs, either, and the projects I'm working on seem to consider type hints less readable than the type specs, but I agree that it might be better to make the type preprocessor opt-in for now. That way people can decide when they want to migrate (since version pinning does not always work, and people might also want some other new feature first).

@keewis
Copy link
Contributor Author

keewis commented Aug 12, 2020

I changed the default to False, but I'm not sure if I got the tests right (the question I have been asking myself is whether it would be better to update the setting or expected ).

Edit: would you consider this a breaking change?

@tk0miya
Copy link
Member

tk0miya commented Aug 13, 2020

Thank you for your update. I don't think this is a breaking change. The numpy type spec has been just released last week. So this does not harm anybody, I believe. Let's enable it after enough matured.

@tk0miya tk0miya merged commit 6e62d33 into sphinx-doc:3.2.x Aug 13, 2020
tk0miya added a commit that referenced this pull request Aug 13, 2020
@keewis keewis deleted the toggle-preprocessor branch August 13, 2020 15:06
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants