[stdlib] Add String.removeprefix and String.removesuffix
#2038
Conversation
|
Hey, @gabrieldemarmiesse thanks for your contribution! The "Changes we accept" section documents change types we know we are okay with accepting, but is not an exhaustive list of all change types we're willing to accept. This change you've submitted is an example of the kind of change we're open to accepting as well :) This change looks good, and thank you for the attention to detail in referencing the Python equivalent of these new methods. Though, a minor point on the naming of these methods. It's true that Mojo is seeking to provide a great onboarding experience to developers familiar with Python, and that means fidelity to Python syntax and libraries in many ways. However, at our current stage we're also looking to improve the consistency of the Mojo standard library, and part of that is a stronger convention for the naming of types and functions. The official Style Guide section on naming documents that Could you please rename these new methods, respectively, to Let me know if you have any questions, and thank you again for your help growing the Mojo🔥 standard library! :) |
gabrieldemarmiesse
force-pushed
the
add_removeprefix_and_removesuffix
branch
from
2efe06d
to
a2a2b9d
Compare
gabrieldemarmiesse
changed the title
|
I'm not especially in favor of deviating from the naming of Python's functions and methods, but this is not necessarily the place to discuss this. I made the change that you asked, we should be good to merge :) |
|
I do agree with consistency and do not copy-paste Python naming. Thank you @ConnorGray for following this! |
|
Isn't the goal of Mojo to be a drop-in replacement for Python? How could that be achieved if Mojo is deviating from Python function names? I'd expect that most users would want to be able to just run their python scripts in the mojo environment and have it work, regardless of whether some functions in python's standard library follow naming conventions. |
|
Mojo's Mojo is not aiming to copy paste CPython. This approach has a lot of flaws, e.g.:
Impression that code from CPython can be directly copied to Mojo is harmful illusion. Migration from Python to Mojo is a task for separate tool. If someone would to use almost drop-in replacement, he probably should look at PyPy or Taichi Lang |
|
If you feel strongly about this, please open an issue so that users can voice their opinions. It will be much easier to discuss than in a pull request |
|
No, pr right now looks good and I would like to prevent IMHO wrong changes |
I don't think it's about copying, it's about having a similar API that integrate seemlessly with existing code, also the manual https://docs.modular.com/mojo/manual/ here, states -> "We also designed Mojo as a superset of Python because we love Python and its community" Small decisions like that, have unexpected consequences -> would have been more coherent to keep the same api (function signatures in this case) Even if String are not str, people would use them interchangeably, it's an abstraction for string maniuplation |
|
And that's why, there should be "_" to keep consistency. I am against messing up like in Python. |
People overtime will probably correct it in the next python versions, so will be good to correct it in mojo at this time afterward, i think there is a distinction to be made, for new code (that doesn't exist in python) and existing code, i do agree that python is messing it up there, but there is a practical & pragmatic trade off there to be considered and discussed. Thank you for the work you're doing ! we have huge expectation from mojo on the next coming years |
|
Hey folks, First off, I want to say we really appreciate the enthusiasm from the community on this topic. We all want Mojo🔥 to be a success, and it's fantastic to see folks engaging, weighing the options, discussing tradeoffs, and working with us towards that success. Secondly, and I apologize to @gabrieldemarmiesse for the churn here, but after some further discussion amongst the standard library team, wider Mojo team, and consideration of the different approaches community members here have outlined, we think it is probably best for the time being if we follow the Python naming conventions for these particular functions, and revert to the intuitive naming this PR originally chose: I want to apologize to the community as well for the mixed messaging. This is one of many nuanced design choices we and community will need to navigate as we work to build out the standard library as Mojo continues to mature. We discovered here that we have some work to do regarding naming. While we have long had strong Python interoperability/compatibility as a guiding "north star", we're learning alongside the community about how we should apply and weigh that principle to individual small design decisions. To that end, we'll be continuing to discuss and improve our current Style Guide, especially in relation to issues like this that arise as we and the community grow. Thank you again to everyone who contributed to this discussion. Mojo🔥 is still young, and there's a lot of exciting work to be done, and we're looking forward to learning and growing with all of you :). Special thanks to @gabrieldemarmiesse for the PR, and bearing with us as we iron out these growing pains. |
|
@ConnorGray it's a pitty that Mojo will sacrifice its consistency to copy Python functionalities.. If Python will rename these, then Mojo should immediately follow these changes... |
Signed-off-by: gabrieldemarmiesse <gabrieldemarmiesse@gmail.com>
gabrieldemarmiesse
force-pushed
the
add_removeprefix_and_removesuffix
branch
from
a2a2b9d
to
4600f00
Compare
gabrieldemarmiesse
changed the title
|
Hey @ConnorGray, thanks a lot and no worries, I totally understand that there is some back and forth. The language is evolving so fast, it would be surprising if that didn't happen. I agree with the final decision and adapted the PR accordingly. It seems that the community has strong feelings about this, and it should be able to voice them. To do so, I've created an issue, which would be a good place to continue the discussion: #2113 |
JoeLoser
changed the title
String.removeprefix and String.removesuffix
JoeLoser
changed the title
String.removeprefix and String.removesuffixString.removeprefix and String.removesuffix
…ml#2038) The two methods `removeprefix` and `removesuffix` are present in python. See the docs: * https://docs.python.org/3/library/stdtypes.html#str.removeprefix * https://docs.python.org/3/library/stdtypes.html#str.removesuffix Both were introduced in python 3.9. The documentation was added, the arguments were made positional-only like in CPython. Unit tests were added in the corresponding module. The docstrings were taken from the python documentation. Signed-off-by: gabrieldemarmiesse <gabrieldemarmiesse@gmail.com>
…ml#2038) The two methods `removeprefix` and `removesuffix` are present in python. See the docs: * https://docs.python.org/3/library/stdtypes.html#str.removeprefix * https://docs.python.org/3/library/stdtypes.html#str.removesuffix Both were introduced in python 3.9. The documentation was added, the arguments were made positional-only like in CPython. Unit tests were added in the corresponding module. The docstrings were taken from the python documentation. Signed-off-by: gabrieldemarmiesse <gabrieldemarmiesse@gmail.com>
…ml#2038) The two methods `removeprefix` and `removesuffix` are present in python. See the docs: * https://docs.python.org/3/library/stdtypes.html#str.removeprefix * https://docs.python.org/3/library/stdtypes.html#str.removesuffix Both were introduced in python 3.9. The documentation was added, the arguments were made positional-only like in CPython. Unit tests were added in the corresponding module. The docstrings were taken from the python documentation. Signed-off-by: gabrieldemarmiesse <gabrieldemarmiesse@gmail.com>
…ml#2038) The two methods `removeprefix` and `removesuffix` are present in python. See the docs: * https://docs.python.org/3/library/stdtypes.html#str.removeprefix * https://docs.python.org/3/library/stdtypes.html#str.removesuffix Both were introduced in python 3.9. The documentation was added, the arguments were made positional-only like in CPython. Unit tests were added in the corresponding module. The docstrings were taken from the python documentation. Signed-off-by: gabrieldemarmiesse <gabrieldemarmiesse@gmail.com>
…ce` (modularml#2035) Replace most uses of `__get_lvalue_as_address` with `Reference`. The two remaining uses at `memory/anypointer.mojo` and `memory/unsafe.mojo` can't switch over to using `Reference` since register passable things don't carry a lifetime value (and thus cannot be used with `Reference`). They are current rejected by the compiler with the error: ```mojo error: cannot form a reference to an argument that might instantiate to @register_passable type ``` Fixes modularml#2027. Signed-off-by: Shaikh Ubaid <shaikhubaid769@gmail.com> (+19 squashed commits) Squashed commits: [64928ee] [Utils] Remove doc string scripts (modularml#2118) These doc strings scripts for doing API doc string validation don't yet work. Remove them for now to avoid any confusion for other contributors. [d0b6eae] [stdlib] Use variadic initializer for `List` in tests (modularml#2074) Update the tests to make use of the variadic initializer of `List`. Signed-off-by: Mert Celik <otuz.bes.bucuk@hotmail.com> [3eb5d13] [stdlib] Add `String.removeprefix` and `String.removesuffix` (modularml#2038) The two methods `removeprefix` and `removesuffix` are present in python. See the docs: * https://docs.python.org/3/library/stdtypes.html#str.removeprefix * https://docs.python.org/3/library/stdtypes.html#str.removesuffix Both were introduced in python 3.9. The documentation was added, the arguments were made positional-only like in CPython. Unit tests were added in the corresponding module. The docstrings were taken from the python documentation. Signed-off-by: gabrieldemarmiesse <gabrieldemarmiesse@gmail.com> [9a18569] [stdlib] Check compiler versions before building There's often breaking changes in the compiler that mandate an update to the latest `nightly/mojo` compiler in order to work with the latest standard library. Currently, if a user installs a newer `nightly/mojo` compiler but hasn't updated the library source code, they'll get pages of bad errors depending on the breaking change, often "cryptic" MLIR errors. This is not a great user experience, and does not tell contributors what they can/should do. To improve this user experience, check the current installed compiler version in the script when building the standard library. If there is a mismatch between the expected compiler version (stored in a file in the repo) and the installed compiler, give a good error message telling the user to update their `nightly/mojo` compiler. Note: - An alternative approach is to query to find the "latest compiler version released" and check against that, but that has the caveat that just because internally we publish a new `nightly/mojo` package, if someone hasn't updated their local copy of the standard library source code, they shouldn't be prevented from building just because something out-of-band changed/got updated. This approach keeps the local flow working until they update their copy of the library source code. - We'll probably want to rewrite these bash scripts into Python for a bit better maintenance soon. [ffb784d] [mojo-lang] Expand `VariadicPack` to take parametric trait base (#36319) This patch is a big step towards enabling VariadicPack to model packs with a non-AnyType bound (e.g. all members must be Stringable). It expands `VariadicPack` with a new `element_trait` member that indicates what all the elements are, and teaches the parser to pass down the element type. The magic enabling this is support for metatype bound type expressions which is almost working, but not quite there yet. As such, this is another step, but isn't enough to declare success, which is why the integration test isn't using it yet. :-/ modular-orig-commit: d45b597eef0f58fc20770c2b7ec860216e9ecc6c [400acec] [mojo][repl] Fix erroneous destructor call emission in generated REPL source (#35595) The REPL is doing dirty tricks with variables, turning them into heap allocations and tracking them in indirect structs. However, CheckLifetimes is onto its tricks, which causes it to reject these as incorrect values. The right solution is to make more invasive changes to the REPL, but that isn't in the short term plans. Workaround this by introducing a horrible hack (tm) op `RefFromPointerUntrackedOp` that creates a reference that isn't tracked by CheckLifetimes. This pokes a hole in our nice reference system which makes me very sad. A comment saying "don't use this" will prevent other people from using this for other things ... right??? --------- Co-authored-by: Chris Lattner <clattner@nondot.org> modular-orig-commit: 7a6b7339522d2657f72d7e8a3a5b51e4d299167f [55c27f3] [mojo-stdlib] Move DTypePointer inits to `inout Self` (#36262) This is split off of #33595 modular-orig-commit: d745e5673d768ab862661f0ada2adb91f34e81e2 [bdc32af] [Internal Change] modular-orig-commit: 44d3a3560edfbd4510e7a906d895a5b01c04b606 [0ecd8f1] [docs] Cross-link the vision and roadmap docs (#36142) Maybe that wasn't the doc you were looking for. modular-orig-commit: fb7d62c22be4e0edf05c49e4624c9fa4a1f880f8 [e79c05c] [CHANGELOG] Update for `mojo build` change (#36210) Update the changelog to reflect #36207, since it represents a slight change in the `mojo` tool's behavior. modular-orig-commit: 0e954f4d4768c3674dfd4f8989f67050bec140cf [3a46831] [mojo-examples] Fix incorrect logic in vectorize loop (#36196) The logic was incorrect if there was a remainder from C.cols % nelts modular-orig-commit: 347157c2401cdacd24f97f1a42027245438e168d [2579983] [Stdlib] Reconcile the div_ceil and ceildiv functions (#36203) We only need a single one of these, so just remove the div_ceil function in place of the ceildiv one (since the ceildiv is what it's called in Python). Closes #33323 modular-orig-commit: 627e5eef1aa20d7c03f0330ad608b2dbbe24a945 [423d7ea] [mojo-driver] Add a `-g` option that aliases `--debug-level full` (#36069) This provides a more familiar way to enable full debugging that matches many other languages. Closes #24171 modular-orig-commit: 7d9038b935bbf86b775237a33b0cb294798200af [6c5b7f7] [mojo-stdlib] Remove always_inline from debug_assert (#36065) This was required originally because of issues related to packaging, but with the removal of separate package codegen, this is no longer necessary. Closes #24957 modular-orig-commit: 79be2c2c74f2aefac95625115a8b7baf291c25ad [c829aad] [mojo] Error out on defining `main` in a package (#36062) The semantics of this are not well defined for mojo right now, so it's cleaner to just disallow it. We can loosen this up when we have cleaner semantics around entry points in packages. Closes [Internal Link] modular-orig-commit: 3203283b41bfd984cd183ba1a314c0c3ba13925d [5d0bd69] [mojo-lang] Change mlir-ification of values of trait type. (#36166) This changes substituteMLIRMagic to include the trait type in the expanded string for a value of trait type in a KGEN type context. This makes it easier and less error prone to work with trait values. If there is a reason to get the raw type without a preceding type, then you can use the unary plus hack like for other attribute values. modular-orig-commit: 09c8a037b73d016d139e8391bb37d31c943ed9b4 [b694374] [mojo-stdlib] Move a few more regpassable inits to 'inout self'. (#36150) This just cleans up a few more of these in builtin_list. modular-orig-commit: 0f88978e8005a0e0aae4c2e279b57cbca8b101c7 [1fe6968] [Docs] Update load()/store() item and move it to top of removed section. (#36110) modular-orig-commit: 038e34dca15cff2a2bdb22285f6d90628737a66d [915e71e] Fix small typo time.mojo (modularml#2067) * Update time.mojo * Update time.mojo
Apologies since adding methods is not listed as "Changes we accept" in the readme. I had already implemented those methods in my own repo and it was very low effort to submit a first PR. Furthermore I believe adding those two methods is not controversial
and has a very low bikeshedding potential.
The two methods
removeprefixandremovesuffixare present in python. See the docs:Both were introduced in python 3.9.
The documentation was added, the argments were made positional-only like in CPython.
Unit tests were added in the corresponding module.
The docstrings were taken from the python documentation.