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

Context: Filter MarkupResemblesLocatorWarning from BeautifulSoup #46

Merged
merged 1 commit into from Apr 15, 2024

Conversation

SmithChart
Copy link
Member

BeautifulSoup will raise a MarkupResemblesLocatorWarning every time it is asked to parse a string that looks like a path or url.

This warning is intended to aid novice users. But in the context of flamingo it leads to warning every time a content consists of something that looks like a path or url to bs4.

With this change we globally disable this specific warning from bs4. Disabling warning this early in the startup process makes this effective for flamingo itself and all plugins. (Settings are loaded earlier - but I hope nobody needs to run bs4 in their settings modules...)

@SmithChart SmithChart self-assigned this Apr 5, 2024
Copy link

codecov bot commented Apr 5, 2024

Codecov Report

Attention: Patch coverage is 33.33333% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 50.65%. Comparing base (1c6edfc) to head (ea5e010).

❗ Current head ea5e010 differs from pull request most recent head 85f817a. Consider uploading reports for the commit 85f817a to get more accurate results

Files Patch % Lines
flamingo/core/context.py 33.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #46      +/-   ##
==========================================
- Coverage   50.66%   50.65%   -0.02%     
==========================================
  Files          66       66              
  Lines        4141     4144       +3     
  Branches      821      821              
==========================================
+ Hits         2098     2099       +1     
- Misses       1835     1837       +2     
  Partials      208      208              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Bastian-Krause
Copy link
Member

The approach looks good to me. Other projects that filter this warning pass a module= argument to warnings.filterwarnings(). Maybe that helps avoiding the (theoretical) plugin use case you described?

BeautifulSoup will raise a `MarkupResemblesLocatorWarning` every time it
is asked to parse a string that looks like a path or url.

This warning is intended to aid novice users. But in the context of
flamingo it leads to warning every time a content consists of something
that looks like a path or url to bs4.

With this change we globally disable this specific warning from bs4.
Disabling warning this early in the startup process makes this effective
for flamingo itself and all plugins. (Settings are loaded earlier - but
I hope nobody needs to run bs4 in their settings modules...)

Signed-off-by: Chris Fiege <cfi@pengutronix.de>
@SmithChart
Copy link
Member Author

The approach looks good to me. Other projects that filter this warning pass a module= argument to warnings.filterwarnings(). Maybe that helps avoiding the (theoretical) plugin use case you described?

I thought about that, too. But that would mean, that I would have to add a filter for every plugin that may provoke this warning.
To be honest: I do not think that this warning is, in the context of flamingo, useful at all. So deactivating it completely seems like the simplest solution.

Copy link
Member

@Bastian-Krause Bastian-Krause left a comment

Choose a reason for hiding this comment

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

The approach looks good to me. Other projects that filter this warning pass a module= argument to warnings.filterwarnings(). Maybe that helps avoiding the (theoretical) plugin use case you described?

I thought about that, too. But that would mean, that I would have to add a filter for every plugin that may provoke this warning. To be honest: I do not think that this warning is, in the context of flamingo, useful at all. So deactivating it completely seems like the simplest solution.

Ah right, agreed. Then, I'm happy with it as is.

@SmithChart SmithChart merged commit 030a4b4 into pengutronix:master Apr 15, 2024
7 checks passed
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

2 participants