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

Jump to bottom

[Feature Request] Consolidate bin() and hex() implementations #2730

Closed
1 task done
bgreni opened this issue May 18, 2024 · 2 comments
Closed
1 task done

[Feature Request] Consolidate bin() and hex() implementations #2730

bgreni opened this issue May 18, 2024 · 2 comments
Labels
enhancement New feature or request help wanted Extra attention is needed mojo-repo Tag all issues with this label

Comments

@bgreni
Copy link

bgreni commented May 18, 2024

Review Mojo's priorities

What is your request?

Immediately upon completing #2603 I realized hex.mojo already contains a general implementation for such things. These should be consolidated into a single implementation that accepts any size of SIMD or Indexer type. Having done this it would be trivial to add oct as well.

What is your motivation for this change?

Reduce code duplication.

Any other details?

No response
@bgreni bgreni added enhancement New feature or request mojo-repo Tag all issues with this label labels May 18, 2024
@JoeLoser JoeLoser added the help wanted Extra attention is needed label May 21, 2024 — with Linear
@msaelices
Copy link

Can I work on this?

@bgreni bgreni mentioned this issue May 22, 2024
Closed
@bgreni
Copy link
Author

bgreni commented May 22, 2024

Can I work on this?

Apologies for not opening a draft sooner but I've just put up a PR for it #2790

modularbot pushed a commit that referenced this issue May 31, 2024
@bgreni @rparolin @modularbot
[External] [stdlib] Consolidate bin and hex internal implementation (…
2729b02 
…#40690)

[External] [stdlib] Consolidate bin and hex internal implementation

Fixes #2730

- Utilize `_format_int` for for both `bin` and `hex`
- Support `SIMD` and `Indexer` types for `hex`
- Move implementations into `format_int` module

---------

Co-authored-by: bgreni <42788181+bgreni@users.noreply.github.com>
Co-authored-by: Rob Parolin <rparolin@modular.com>
Closes #2790
MODULAR_ORIG_COMMIT_REV_ID: 0d5c716cb884468c8d3d49ac585115c39f131880
@bgreni bgreni mentioned this issue Jun 1, 2024
Open
@bgreni bgreni closed this as completed Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request help wanted Extra attention is needed mojo-repo Tag all issues with this label
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@msaelices @JoeLoser @bgreni