Third option of searching HTML content for the price#3414
Third option of searching HTML content for the price#3414louispires wants to merge 5 commits intodgtlmoon:masterfrom
Conversation
Process price with thousand separators (comma)
|
I only just saw #3342 - Which could be combined with my PR |
| {{ watch['restock']['price']|format_number_locale }} {{ watch['restock']['currency'] }} | ||
| </span> | ||
| <span class="restock-label price" title="Price"> | ||
| {{ watch['restock']['currency'] }} {{ watch['restock']['price']|format_number_locale }} |
There was a problem hiding this comment.
in general the locale should dictate if the currency goes before or after, is that what this does?
There was a problem hiding this comment.
Unfortunately, not. This isn't a big deal at all, I just had many scenarios where it would show:
In Stock 499.00 $
In Stock 2000.00 R
In Stock 3000.00 ZAR
It just looked cleaner to have it as follows:
In Stock $ 499.00
In Stock R 2000.00
In Stock ZAR 3000.00
| return list(unique_data) | ||
|
|
||
|
|
||
| def extract_price_from_html(html_content): |
There was a problem hiding this comment.
I think this is a really problematic approach :( many websites mention discount, shipping, specials etcetc
There was a problem hiding this comment.
Totally understand, however since this will only run if the first 2 options failed - Thus it might give wrong information in certain scenarios, but in these scenarios, it wouldn't have provided any information anyway.
I have 3 sites that wouldn't work with any of the current methods but now are fully functional with this additional approach - Also sites that worked before, are NOT affected by this change at all.
There was a problem hiding this comment.
I'm a firm believer that "no information" is better than wrong information, because generally speaking it will be me who will have to deal with the new issues in the queue, it's a pattern that if I accept such a PR, there's no guarantee from the person who wrote it that they will support this code in the future on this project :)
|
basic linting didnt pass, looks like the python source code format is broke |
Fixed in the latest commit |
|
I would actually be in favour of adding some 'pluggy' support to the price detector, then in |
2 participants
_deduplicate_pricesto also be able to process prices with Thousand separatorwatch-overview.htmlto show Currency in front of the Priceextract_price_from_htmlmethod which will search for the Price via the HTML content