IDLE: pyparse - simplify StringTranslatePseudoMapping

[csabella]

BPO	32940
Nosy	@terryjreedy, @serhiy-storchaka, @csabella, @miss-islington
PRs	bpo-32940: IDLE: in pyparse, replace mapping class with defaultdict #5854 bpo-32940: IDLE: Simplify StringTranslatePseudoMapping in pyparse #5862 [3.7] bpo-32940: IDLE: Simplify StringTranslatePseudoMapping in pyparse (GH-5862) #5940 [3.6] bpo-32940: IDLE: Simplify StringTranslatePseudoMapping in pyparse (GH-5862) #5941

^{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 = 'https://github.com/terryjreedy'
closed_at = None
created_at = <Date 2018-02-24.17:38:40.168>
labels = ['3.8', 'expert-IDLE', 'type-feature', '3.7']
title = 'IDLE: pyparse - simplify StringTranslatePseudoMapping'
updated_at = <Date 2018-02-28.23:12:17.496>
user = 'https://github.com/csabella'

bugs.python.org fields:

activity = <Date 2018-02-28.23:12:17.496>
actor = 'miss-islington'
assignee = 'terry.reedy'
closed = False
closed_date = None
closer = None
components = ['IDLE']
creation = <Date 2018-02-24.17:38:40.168>
creator = 'cheryl.sabella'
dependencies = []
files = []
hgrepos = []
issue_num = 32940
keywords = ['patch']
message_count = 15.0
messages = ['312734', '312752', '312764', '312768', '312770', '312778', '312796', '312799', '313046', '313048', '313068', '313071', '313072', '313073', '313074']
nosy_count = 4.0
nosy_names = ['terry.reedy', 'serhiy.storchaka', 'cheryl.sabella', 'miss-islington']
pr_nums = ['5854', '5862', '5940', '5941']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue32940'
versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']

[csabella]

Based on timing test on msg312733, StringTranslatePseudoMappingTest and a defaultdict have about the same performance, so this replaces the custom class with the stdlib functionality.

[csabella]

Tal had written this on the original bpo-21765:
----
Finally, since the defaultdict is kept around as long as IDLE is running, I decided to avoid having it grow continually and consume memory unnecessarily. So I wrote a simple Mapping class, which wraps a normal dict and uses a custom default value instead of None, ord('x') in this case. Works like a charm :)
-----
So maybe I misunderstood and this shouldn't be a defaultdict?

[terryjreedy]

I forget about this defaultdict behavior: "this value is inserted in the dictionary for the key, and returned." Reason: when default_factory returns a mutable, d[key] must return the same possibly mutated object with each call. I agree that defaultdict is not the right replacement.

We need to pass to str.translate a dict that can be used by subscripting, newchar = d[char]. So partial(non-defaults.get, default_value) will not work. Instead, we need a __getitem__ that returns the same.

In msg312444 I suggested simplifying STPM (including the name) because it has unneeded complexity. Remove the buggy .get override. Combine the _get stuff in __init__ (also removed) with current __getitem__ and simplify and we get what we actually need (untested, at yet).

def __getitem__
return self._non_defaults.get(self._default_value)

Actually, we could hard-code the default value as 'X' as we never need anything else.

How about ParseMap for the name?

[csabella]

New PR submitted. Really glad I worked on this today. I learned a lot of things that were new to me. :-)

[terryjreedy]

I simplified ParseMap to a dict subclass with one override -- __getitem__ and then the tests. They run faster. I suspect translate is faster.

[serhiy-storchaka]

For efficiency I suggest to initialize the mapping with dict.fromkeys(range(128), 'x') rather of an empty dict.

It is also possible to use regular expressions:

_trans = re.compile(r'''[^(){}\[]"'\\\n#]+''')
code = _trans.sub('x', code)
code = code.replace('{', '(')
code = code.replace('}', ')')
code = code.replace('[', '(')
code = code.replace(']', '(')
code = code.replace('\nx', '\n')

I didn't check what way is more efficient.

[terryjreedy]

To me, it is plausible but not slam-dunk obvious that preloading ascii to 'x' mappings will make ascii lookup faster.

On bpo-21765, where the pyparse special translation was a side-issue, Tal Einat claimed that the unpublished regex he tried was 100x slower.

[csabella]

A similar regular expression version was mentioned on bpo-21765 and I had run some tests on it yesterday to verify. On my system, it ran at a factor of 10x slower, so if the translate finished in 0.003, the regex took 0.03. This was consistent for me, regardless of how big I made the document.

The reason for not using a defauldict was to keep the 'x' mappings out of the dictionary so that it wouldn't grow and take up space. Although, I did realize yesterday that it wasn't really boundless because most values in source code would be ASCII. Running both the version the doesn't add the 'x' mappings and the fromkeys, there doesn't seem to be much of a difference in time when processing the doc.