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

EAN13 calculate_checksum() generates inconsistent values #196

Closed
zspherez opened this issue Mar 27, 2023 · 6 comments
Closed

EAN13 calculate_checksum() generates inconsistent values #196

zspherez opened this issue Mar 27, 2023 · 6 comments

Comments

@zspherez
Copy link

When using the EAN13() constructor with a 12 digit argument to create an EAN13, the check digit generated in the constructor is different than if you were to call calculate_checksum() on the 13 digit value returned by the constructor.

>>> EAN13("842169142322")
<EuropeanArticleNumber13('8421691423220')>

gives that the check digit is 0

>>> ean = EAN13("842169142322")
>>> ean
<EuropeanArticleNumber13('8421691423220')>
>>> ean.calculate_checksum()
4

implying that the check digit is 4

This seems to be due to the fact that when calculate_checksum() is called on a 13 digit EAN via the object itself, the reverse traversal that is used in the method includes the 13th digit when summing and calculating the check digit.

@stonebig
Copy link

stonebig commented Jun 3, 2023

hum,

on my own method using the generic check digits algorithm given per GS/1, I get the same result as python-barcode:

image

so maybe this shall be "rebranded" as "ean_key_computation", but it's not faulty as you think

@WhyNotHugo
Copy link
Owner

The issue is that the implementation saves the whole number plus the checksum into self.ean, but then calculate_checksum calculates the checksum for self.ean (e.g.: a string which already includes the checksum).

@WhyNotHugo
Copy link
Owner

While this is the "expected behaviour" as per the documentation, it's obviously un-intuitive and can be improved. I'm just not sure how to improve it without breaking existing usages.

@WhyNotHugo
Copy link
Owner

I think that #215 should fix it. Can you confirm?

WhyNotHugo added a commit that referenced this issue Sep 20, 2023
@WhyNotHugo
Copy link
Owner

Fixed in #213

@WhyNotHugo
Copy link
Owner

f242961 fixes EAN14

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 a pull request may close this issue.

3 participants