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

Show DeprecationWarnings as INFO messages #2907

Merged
merged 1 commit into from
Aug 1, 2022
Merged

Conversation

oprypin
Copy link
Contributor

@oprypin oprypin commented Jul 31, 2022

(previously they were not shown at all)

This works for both extensions and plugins -- and for MkDocs itself.
It will enable plugin developers to see warnings in advance. Or users to report such warnings to plugin developers.

Ref #2892, Python-Markdown/markdown#1277

Example session:

$ .venv/bin/pip install 'mdx_gh_links==0.2' 'Markdown==3.3.7'
$ PYTHONPATH=. .venv/bin/mkdocs serve                        
INFO     -  Building documentation...
INFO     -  DeprecationWarning: 'etree' is deprecated. Use 'xml.etree.ElementTree' instead.
              File "/home/blaxpirit/repos/mkdocs/.venv/lib/python3.10/site-packages/mdx_gh_links.py", line 35, in <module>
                from markdown.util import etree
              File "/home/blaxpirit/repos/mkdocs/.venv/lib/python3.10/site-packages/markdown/util.py", line 475, in __getattr__
                warnings.warn(
INFO     -  DeprecationWarning: Using setitem to register a processor or pattern is deprecated. Use the `register` method instead.
              File "/home/blaxpirit/repos/mkdocs/.venv/lib/python3.10/site-packages/mdx_gh_links.py", line 126, in extendMarkdown
                md.inlinePatterns['issue'] = IssuePattern(self.getConfigs(), md)
              File "/home/blaxpirit/repos/mkdocs/.venv/lib/python3.10/site-packages/markdown/util.py", line 393, in __setitem__
                warnings.warn(
INFO     -  DeprecationWarning: The 'md_globals' parameter of 'mdx_gh_links.GithubLinks.extendMarkdown' is deprecated.
              File "/home/blaxpirit/repos/mkdocs/.venv/lib/python3.10/site-packages/markdown/core.py", line 125, in registerExtensions
                ext._extendMarkdown(self)
              File "/home/blaxpirit/repos/mkdocs/.venv/lib/python3.10/site-packages/markdown/extensions/__init__.py", line 82, in _extendMarkdown
                warnings.warn(
INFO     -  Cleaning site directory
INFO     -  Documentation built in 0.51 seconds

cc @ultrabug @pawamoy

(previously they were not shown at all)

This works for both extensions and plugins -- and for MkDocs itself.
It will enable plugin developers to see warnings in advance. Or users to report such warnings to plugin developers.
Copy link
Member

@ultrabug ultrabug left a comment

Choose a reason for hiding this comment

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

Good job, I find that very elegant

@pawamoy
Copy link
Sponsor Contributor

pawamoy commented Aug 1, 2022

Just to understand: the change basically makes sure warnings are shown to the user by logging them as INFO? If users already have warnings shown, will they see them twice (once as a regular warning and a second time as an info log)? Also, good call on making them INFO level and not WARNING 👍

@oprypin
Copy link
Contributor Author

oprypin commented Aug 1, 2022

There are 2 mechanisms at play here, both of them configured totally separately and both of them affected by this PR:

  1. Whether a warning is enabled.
    Regardless of how the environment is configured, this forcibly enables all DeprecationWarnings.
    If a warning is enabled, it proceeds to the "show warning" part.

  2. How to show a warning.
    This forcibly replaces the way of displaying all warnings - to go to the log mechanism instead of the default output.
    There is no way for the default mechanism to kick in anymore.
    This is the recommended approach https://docs.python.org/3/library/warnings.html#warnings.showwarning

@pawamoy
Copy link
Sponsor Contributor

pawamoy commented Aug 1, 2022

Thanks, that answers my questions :) perfect.

@oprypin oprypin merged commit 3ffe21d into mkdocs:master Aug 1, 2022
@pawamoy
Copy link
Sponsor Contributor

pawamoy commented Aug 1, 2022

Actually I'm not sure if hiding regular warnings in favor of INFO logs is a good idea. I mean, maybe keeping the duplication would be better. In the common case, that's a few more lines of INFO logs few will read, in the worst case, users that were relying on regular warnings don't get their pytest run fail anymore, and I believe that's less chance these users will report these deprecations to the upstream packages. I hope I'm wrong though. The INFO logs are pretty visible so I guess it's fine.

@oprypin
Copy link
Contributor Author

oprypin commented Aug 1, 2022

users that were relying on regular warnings don't get their pytest run fail anymore,

Why would that happen? This affects only full runs of mkdocs

I mean, maybe keeping the duplication would be better.

How is printing the exact same message twice better? There is no possible case where only one of them would kick in - it's either 2 or 0.

@oprypin
Copy link
Contributor Author

oprypin commented Aug 1, 2022

There is no possible case

Well, mkdocs -q hides these messages, but that's a desired thing.

Other than that, there's no difference, both are just text in stderr

@pawamoy
Copy link
Sponsor Contributor

pawamoy commented Aug 1, 2022

Hmmm yeah I don't think a lot of users/plugin developers run MkDocs commands through pytest with warnings enabled anyway 🤔
Always happy to be proven wrong.
I just know warnings are a nice way to, well, warn users, and worry about the effects of replacing them with logs, which don't react to the same mechanisms. But I'm probably overthinking here. I trust your decisions @oprypin. And I'm slowly getting back to open source so don't mind me too much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants