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

[Feature Request] polymorphic string functions, like isdigit() #2975

Open
1 task done
dimitrilw opened this issue Jun 6, 2024 · 7 comments
Open
1 task done

[Feature Request] polymorphic string functions, like isdigit() #2975

dimitrilw opened this issue Jun 6, 2024 · 7 comments
Labels
enhancement New feature or request mojo-python-interop mojo-repo Tag all issues with this label

Comments

@dimitrilw
Copy link

Review Mojo's priorities

What is your request?

Right now, isdigit() (found here) expects to receive a numeric value. I.e., the two noted lines fail:

image

But if I pass ord(<value>), then it works:

image

In Python, the str.isdigit() method is called from the string itself. More, it does not require advance alteration of the input.

image

Perhaps Mojo's isdigit() function should be polymorphic to take different inputs. For example, in Python, it can assess a full string of characters to confirm all are digits:

image

I'm thinking that the Mojo function should perform similarly. Not necessarily as a String.method(), but rather just something that can take different inputs, to include something like isdigit("1x3"), which would evaluate as False.

If Modular peeps agree, then I will gladly work to make this real and make a PR.

Note, the same could be said about other functions: islower, isupper, etc.

Thoughts?

What is your motivation for this change?

Pythonic style to simplify onboarding Py-Peeps. Also, easier-to-read code; I believe isdigit("x") is more intuitive than isdigit(ord("x")).

Any other details?

No response

@dimitrilw dimitrilw added enhancement New feature or request mojo-repo Tag all issues with this label labels Jun 6, 2024
modularbot pushed a commit that referenced this issue Jun 15, 2024
[External] [stdlib] Add `String.isdigit`

Partially addresses #2975

- Added convenience `String` overload `isdigit` function
- Add `isdigit` member to `String` that checks if all characters in a
string are digits as python has

Co-authored-by: bgreni <42788181+bgreni@users.noreply.github.com>
Closes #3027
MODULAR_ORIG_COMMIT_REV_ID: ef774445f416c57af49885d289f53847a145418e
@dimitrilw
Copy link
Author

I have a fork that adds:

  • isupper(String)
  • islower(String)
  • String.isupper()
  • String.islower()

It follows the same pattern as bgreni's isdigit additions.

I also added tests for each of those additions, but want to ensure I pass all tests locally before submitting the PR. I could use a little hand-holding on "here's how you recompile Mojo locally from your local fork and ensure your system uses that mojo binary to run the test suite."

@dimitrilw
Copy link
Author

PR to close this ticket when merged: #3095

@soraros
Copy link
Contributor

soraros commented Jun 21, 2024

isdigit(Int8) is actually isdigit(Char). Mojo used to use signed 8bit integer for char.

@dimitrilw
Copy link
Author

Not sure I follow.

The function signature at the time I originally wrote this was only one: fn isdigit(c: UInt8) -> Bool:. Then bgreni added fn isdigit(c: String) -> Bool:, which is a pass-through to the original function.

@soraros
Copy link
Contributor

soraros commented Jun 22, 2024

I was merely pointing out that one should think of the "numeric" overload as a function on char.

@dimitrilw
Copy link
Author

dimitrilw commented Jun 22, 2024

Tracking. ❤️

@dimitrilw
Copy link
Author

For ref, python 3 str.islower() and str.isupper():
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request mojo-python-interop mojo-repo Tag all issues with this label
Projects
None yet
Development

No branches or pull requests

3 participants