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

Fix bug where a module import set up logging, pin doctools for Py2 #2552

Merged
merged 5 commits into from
Aug 26, 2019

Conversation

piskvorky
Copy link
Owner

@piskvorky piskvorky commented Jul 4, 2019

  • Module imports must never issue logging events, because logging is not set up in the main app yet. The logging event triggered a default (unwanted) config init.
  • warnings.warn better, but in this case, we can more simply re-use the existing functionality from gensim.utils

The bug manifested itself as an unexpected log output on Gensim import:

 2019-07-04 23:22:48,902 [MainProcess/98263] [INFO] gensim.summarization.textcleaner:37: 'pattern' package not found; tag filters are not available for English

This may be a deeper issue with the summarization subpackage doing weird stuff. This PR is only a surface stop-gap, I didn't look any deeper.

- module imports must never issue logging events (logging not set up yet in main app, triggers a default config init)
- warnings better, but in this case, we can more simply re-use the existing functionality from `gensim.utils`
@piskvorky piskvorky added the bug Issue described a bug label Jul 4, 2019
@piskvorky piskvorky requested a review from mpenkov July 17, 2019 17:35
@mpenkov mpenkov self-assigned this Jul 21, 2019
@mpenkov
Copy link
Collaborator

mpenkov commented Jul 21, 2019

Looks like the tests are failing. Did you forget to check some changes in? Or is this a work in progress?

@piskvorky
Copy link
Owner Author

piskvorky commented Jul 21, 2019

I didn't run it, just edited the faulty file on Github, to get rid of the logging setup. Good we have the tests!

Edited some more now, hopefully that fixes it.

@mpenkov
Copy link
Collaborator

mpenkov commented Jul 22, 2019

Tests are still failing, but the cause seems to be some Py2 bs with one of our dependencies.

@piskvorky
Copy link
Owner Author

According to pywbem/pywbem#1788, pinning docutils to 0.14 helps… not sure where to do that though. Is docutils some internal dependency of Sphinx?

@mpenkov
Copy link
Collaborator

mpenkov commented Aug 26, 2019

Finally got around to looking at this. Pinned doctools in setup.py, let's see if it helps.

@mpenkov mpenkov changed the title Fix bug where a module import set up logging Fix bug where a module import set up logging, pin doctools for Py2 Aug 26, 2019
@mpenkov mpenkov merged commit 4c8be8b into develop Aug 26, 2019
@mpenkov mpenkov deleted the piskvorky-patch-1 branch August 26, 2019 23:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue described a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants