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

Add str.replace(replacement_map) #77828

Closed
paalped mannequin opened this issue May 25, 2018 · 8 comments
Closed

Add str.replace(replacement_map) #77828

paalped mannequin opened this issue May 25, 2018 · 8 comments
Labels
3.8 only security fixes type-feature A feature request or enhancement

Comments

@paalped
Copy link
Mannequin

paalped mannequin commented May 25, 2018

BPO 33647
Nosy @rhettinger, @terryjreedy, @bitdancer, @cjerdonek, @serhiy-storchaka, @grimreaper, @paalped

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2018-05-25.12:52:17.546>
labels = ['type-feature', '3.8']
title = 'Add str.replace(replacement_map)'
updated_at = <Date 2020-10-08.21:46:34.843>
user = 'https://github.com/paalped'

bugs.python.org fields:

activity = <Date 2020-10-08.21:46:34.843>
actor = 'chris.jerdonek'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = []
creation = <Date 2018-05-25.12:52:17.546>
creator = 'paalped'
dependencies = []
files = []
hgrepos = []
issue_num = 33647
keywords = []
message_count = 7.0
messages = ['317672', '317685', '317710', '317724', '317728', '317736', '378286']
nosy_count = 7.0
nosy_names = ['rhettinger', 'terry.reedy', 'r.david.murray', 'chris.jerdonek', 'serhiy.storchaka', 'eitan.adler', 'paalped']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'test needed'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue33647'
versions = ['Python 3.8']

@paalped
Copy link
Mannequin Author

paalped mannequin commented May 25, 2018

It would be really nice if the python core function string.replace is modifed to accept **kwargs instead of 2 arguments.

then you could just do:

kwargs = {'my': 'yours', 'string': 'car'}

>>'this is my string'.replace(kwargs)
'this is your car'

@paalped paalped mannequin added 3.7 (EOL) end of life 3.8 only security fixes type-feature A feature request or enhancement labels May 25, 2018
@bitdancer
Copy link
Member

That is not kwargs, that's a passing a dict. Which is what you would want, since the strings you want to replace might not be valid identifiers, and so couldn't be passed as keyword arguments.

I think I'm -0.5 on this. I don't think complicating the api is worth the benefit, given that you can already chain replace calls. (And note that before dicts became ordered by language definition this would have been a non-starter, which is probably also a mark against it.)

@bitdancer bitdancer changed the title Make string.replace accept **kwargs instead of two arguments Make string.replace accept a dict instead of two arguments May 25, 2018
@bitdancer bitdancer removed the 3.7 (EOL) end of life label May 25, 2018
@terryjreedy
Copy link
Member

Your proposal is underspecified. (And 'yours' is inconsistent with 'your';-). If you want sequential replacememt in multiple scans, make sequential replace calls. Use a loop if the number of replacement scans is variable or large. To be sure of the order of replacements, a sequence is better than a dict, but dict.values() instead would work in the following code.

s = 'this is my string'
for old, new in (('my', 'your'), ('string', 'car')):
    s = s.replace(old, new)
print(s)

If you want parallel replacement in a single scan, a different scanner is required. If the keys (search strings) are all single letters, one should use str.translate. For a general string to string mapping, use re.sub with a replacement function that does a lookup.

import re

s = 'this is my string'
subs = {'my': 'your', 'string': 'car'}
print(re.sub('|'.join(subs), lambda m: subs[m.group(0)], s))

In any case, the proposal to modify str.replace should be rejected.

However, the re code is not completely trivial (I did not work it out until now), so I think it plausible to add the following to the re module.

def replace(string, map):
    """Return a string with map keys replaced by their values.
*string* is scanned once for non-overlapping occurrences of keys.
"""
return sub('|'.join(map), lambda m: map[m.group(0)], string)

I would reference this in the str.translate entry for when keys are not restricted to letters.

If adding replace() is rejected, I would like an example added to the sub(pattern, function, string) examples.

@terryjreedy terryjreedy changed the title Make string.replace accept a dict instead of two arguments Add re.replace(string, replacement_map) May 25, 2018
@rhettinger
Copy link
Contributor

I would like an example added to the sub(pattern, function,
string) examples.

That seems like the most reasonable way to go.

@serhiy-storchaka
Copy link
Member

I'm -1 of adding support of this in str.replace. This is very non-trivial code, and unicodeobject.c is already one of largest and most complex files. Adding new complex code will make maintaining harder and can make the compiler producing less optimal code for other methods. str.replace is already good optimized, it is often better to call it several times than use other methods (regular expressions or str.translate).

You should be careful with sequential applying str.replace() if some keys are prefixes of other keys ({'a': 'x', 'ab': 'y'}). You should perform replacement in correct order. But this doesn't work either in cases like {'a': 'b', 'b': 'a'}.

The regular expression based implementation should be more complex than Terry's example:

def re_replace(string, mapping):
    def repl(m):
        return mapping[m[0]]
    pattern = '|'.join(map(re.escape, sorted(mapping, reverse=True)))
    return re.sub(pattern, repl, string)

And it will be very inefficient, because creating and compiling a pattern is much slower than performing the replacement itself, and it can't be cached. This function would be not very useful for practical purposes. You will need to split it on two parts. First prepare a compiled pattern:

    def repl(m):
        return mapping[m[0]]
    compiled_pattern = re.compile('|'.join(map(re.escape, sorted(mapping, reverse=True))))

And later use it:

    newstring = compiled_pattern.sub(repl, string)

@paalped
Copy link
Mannequin Author

paalped mannequin commented May 26, 2018

I forgot to put the ** in the input.

This is to further explain what I had in mind:

"""The meaning of this file is to show how the replace function can be
enhanced"""

# Today the replace api takes at most 3 arguments, old_string,
new_string, and optional numbers_of_replacements
# My wish is to update the replace to support a fourth optional
**kwargs argument

class Str(object):
    def __init__(self, s=''):
        self.original_str = s
        self.new_str = s
    # name replace2 is to not cause recursion of my function
    def replace2(self, *args, **kwargs):
        # Old api lives here ...
        if len(args) == 2:
            old_str, new_str = args
            # do the old stuff
        elif len(args) == 3:
            old_str, new_str, numbers_of_replacements = args
            #do the old stuff
        # new api begins
        if kwargs:
            for k, v in kwargs.items():
                # this one relies on the old api from string object
                # str original function must differ
                self.new_str = self.new_str.replace(k, v)
            return self.new_str

    def __str__(self):
        return self.original_str

s = Str('my string is not cool')

replacement_dict = {'is': 'could',
            'my': 'your',
            'not': 'be',
            'cool': 'awsome'}

print(s)
s2 = s.replace2(**replacement_dict)
print(s, s2, sep='\n')

If this is a really crappy though, I would like not to be made a fool out
of. I only want to improve, and I'm not crtizing the old api. I just want a
simplier life for everyone :)

Paal

lør. 26. mai 2018 kl. 02:21 skrev Raymond Hettinger <report@bugs.python.org

:

Raymond Hettinger <raymond.hettinger@gmail.com> added the comment:

> I would like an example added to the sub(pattern, function,
> string) examples.

That seems like the most reasonable way to go.

----------
nosy: +rhettinger


Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue33647\>


@cjerdonek
Copy link
Member

Another API option is to use str.replace's existing arguments: str.replace(old, new[, count]). In addition to "old" and "new" being strings, they could also be lists of strings of equal length, similar to how str.maketrans() can accept two strings of equal length. Then, like maketrans(), each string in "old" would be replaced by the corresponding string at the same position in "new."

@cjerdonek cjerdonek changed the title Add re.replace(string, replacement_map) Add str.replace(replacement_map) Oct 8, 2020
@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@iritkatriel
Copy link
Member

There were at least 3 votes against this enhancement.

@iritkatriel iritkatriel closed this as not planned Won't fix, can't repro, duplicate, stale Jul 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.8 only security fixes type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

6 participants