-
Notifications
You must be signed in to change notification settings - Fork 12
Add base16 and base32 encoding example files #393
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
base: master
Are you sure you want to change the base?
Conversation
This script demonstrates reducing output size using base16 encoding instead of the default base64.
This script demonstrates reducing output size using base32 encoding instead of the default base64.
vil02
left a comment
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.
@mathurojus: thanks for creating this PR!
Please run_all_examples.sh to see why the CI will fail. Let me know if you need some support.
Regarding the example files: I am not sure if they are needed. I think that just mentioning in the existing example, that one can use "base16" and "base32" for the encoding should be enough.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #393 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 17 17
Lines 336 336
Branches 26 26
=========================================
Hits 336 336 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hi @vil02, I've addressed all your review comments: ✅ Deleted the 3 new example files (base16_example.py, base16_to_reduce_output_size.py, base32_to_reduce_output_size.py) as you suggested - instead of adding new files, I updated the existing examples. ✅ Updated base85_to_reduce_output_size.py to mention that base16 and base32 encoding can also be used, with a note clarifying that they increase (not reduce) output size. ✅ Resolved all 4 conversation threads related to the incorrect import statements and misleading comments about output size. The changes should fix the CI issues. The import statement in the original files is already correct ( Ready for you to approve the workflows and re-review. 🚀 |
vil02
left a comment
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.
@mathurojus: there is a fundamental problem with this change. Will the code like:
create(_PUZZLE, encoding="base32")work?
No, because this package does not implement the base32 encoding.
You need to add the files like:
and modify bu_configurators.py accordingly.
Please remember about updating the related tests.
This PR adds new examples using base16 and base32 encodings in the examples folder as requested in issue #391 and by @vil02. Includes both standard and output-size reduction variants.
Closes #391