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.

Sign up for GitHub

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

Support cents, fixes #6 #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+4 −2
Open

Support cents, fixes #6 #14

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion price_parser/parser.py
View file
Original file line number Diff line number Diff line change
@@ -68,7 +68,7 @@ def or_regex(symbols: List[str]) -> Pattern:
'SY£', 'LB£', 'CN¥', 'GH₵',

# unique currency symbols
'$', '€', '£', 'zł', 'Zł', 'Kč', '₽', '¥', '￥',
'$', '¢', '€', '£', 'zł', 'Zł', 'Kč', '₽', '¥', '￥',
'฿', 'դր.', 'դր', '₦', '₴', '₱', '৳', '₭', '₪', '﷼', '៛', '₩', '₫', '₡',
'টকা', 'ƒ', '₲', '؋', '₮', 'नेरू', '₨',
'₶', '₾', '֏', 'ރ', '৲', '૱', '௹', '₠', '₢', '₣', '₤', '₧', '₯',
4 changes: 3 additions & 1 deletion tests/test_price_parsing.py
View file
Original file line number Diff line number Diff line change
@@ -80,7 +80,9 @@ def __eq__(self, other):
Example(None, 'AED 8000 (USD 2179)',
'AED', '8000', 8000),
Example(None, '13800 ₶',
'₶', '13800', 13800)
'₶', '13800', 13800),
Example(None, '99¢',
Copy link
Member

Choose a reason for hiding this comment

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

I think 99¢ should result in Price(value=Decimal('0.99'), currency=None), since the cent symbol does not belong to a specific currency.

Copy link
Member

Choose a reason for hiding this comment

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

This one is super-unclear, what to do exactly in such cases, as it may require changing the format. For example, $ also doesn't belong to a specific currency; we're not normalizing this to ISO codes at the moment.

I'm not sure how cases like 1$ 99¢ are handled here - it seems they are not.

Copy link
Contributor Author

@manycoding manycoding Jul 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure how cases like 1$ 99¢ are handled here - it seems they are not.

They are not.

So how we can find what should be what?

Copy link
Member

Choose a reason for hiding this comment

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

Given the current API, what about aiming for:

  • 99¢ → Price(value=Decimal('0.99'), currency=None)
  • 1$ 99¢ → Price(value=Decimal('1.99'), currency='$')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1$ 99¢ Is it a real example? Have we seen this before?

Copy link
Member

Choose a reason for hiding this comment

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

That’s a great question. I assumed it was, but I’ve been searching for a few minutes and all I’ve been able to find are suggestions to use ¢ only up to 99, and never in combination with a regular currency symbol.

Copy link
Member

Choose a reason for hiding this comment

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

Should we still aim for 99¢ → Price(value=Decimal('0.99'), currency=None)? Or do you guys have reservations with that approach?

Copy link
Member

Choose a reason for hiding this comment

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

@Gallaecio from what I can see, this would be the most correct interpretation.

'¢', '99', 99)
]