scrapinghub / price-parser

Extract price amount and currency symbol from a raw text string
BSD 3-Clause "New" or "Revised" License
316 stars 50 forks source link

Add black #17

Open manycoding opened 5 years ago

manycoding commented 5 years ago

This project has special style, but I am for consistency first.

And to save the time on any future styling comments.

codecov[bot] commented 5 years ago

Codecov Report

Merging #17 into master will not change coverage. The diff coverage is 100%.

@@          Coverage Diff          @@
##           master    #17   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           3      3           
  Lines          93     93           
  Branches       21     21           
=====================================
  Hits           93     93
Impacted Files Coverage Δ
price_parser/__init__.py 100% <ø> (ø) :arrow_up:
price_parser/parser.py 100% <100%> (ø) :arrow_up:
price_parser/_currencies.py 100% <100%> (ø) :arrow_up:
kmike commented 5 years ago

hey! I like some of the changes, but don't like others. For example, tests became harder to read - previously in Example convention was "input data on a first line, expected results on a second", and now it is a mess, as all these fields look very similar.

manycoding commented 5 years ago

@kmike On my taste these tests are harder to read anyway - you cannot really collapse them. I prefer traditional params like here https://github.com/scrapinghub/price-parser/pull/18/files#diff-091825c38112ff1dafc081caf2916d27R1991.

At the first, we can put tests in ignore.

Why does this Price string hint work? https://github.com/scrapinghub/price-parser/blob/master/price_parser/parser.py#L28