-
-
Notifications
You must be signed in to change notification settings - Fork 124
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
Update codex.py #56
base: main
Are you sure you want to change the base?
Update codex.py #56
Conversation
Fixes for bug WhyNotHugo#55 writer_options{"add_checksum": False} being ignored for Code39 barcodes. Added class variable to track when the checksum has been added so it can't add twice Moved the check for checksum to the render function Set a default value class variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to make sense to me -- tho I can't see how the checksum got calculated twice. Have you figured out where that happened?
barcode/codex.py
Outdated
@@ -61,7 +61,7 @@ def get_fullcode(self): | |||
return self.code | |||
|
|||
def calculate_checksum(self): | |||
check = sum(code39.MAP[x][0] for x in self.code) % 43 | |||
check = sum([code39.MAP[x][0] for x in self.code]) % 43 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
List comprehension seems redundant here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Completely agree, not sure how that line got changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No I didn't see an example of it being added twice, but if anyone called the render function twice then it would.
Agreed on list comprehension
|
This was changed in the first commit. I moved the checksum code from the class constructor to the render function. The second commit was just to address your previous comments. If you look at the combined changes you can see the new render code. |
Sorry, I'm not seeing it. 😓 |
I think perhaps I have not explained the change very well. I am traveling at the moment, but will take some time and fully document the bug, the fix, what code breaks etc. Thanks for your patience. |
Hey there! Do you have any update? |
Fixes for bug #55
writer_options{"add_checksum": False} being ignored for Code39 barcodes.
Added class variable to track when the checksum has been added so it can't add twice
Moved the check for checksum to the render function
Set a default value class variable.