[issue26765] Factor out common bytes and bytearray implementation

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26765] Factor out common bytes and bytearray implementation

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

I don't know what is wrong with this patch. Backed out just for the case.

--
resolution: fixed -> 
stage: resolved -> patch review
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26765] Factor out common bytes and bytearray implementation

[Roundup Robot]

Roundup Robot added the comment:

New changeset 6c4c0a23fabb by Serhiy Storchaka in branch 'default':
Backed out changeset b0087e17cd5e (issue #26765)
https://hg.python.org/cpython/rev/6c4c0a23fabb

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26765] Factor out common bytes and bytearray implementation

[Roundup Robot]

Roundup Robot added the comment:

New changeset 0638047c0c36 by Serhiy Storchaka in branch 'default':
Issue #26765: Fixed parsing Py_ssize_t arguments on 32-bit Windows.
https://hg.python.org/cpython/rev/0638047c0c36

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26765] Factor out common bytes and bytearray implementation

[Terry J. Reedy]

Terry J. Reedy added the comment:

Checking, I see that 3 of 4 Windows buildbots ran with this patch, so there 
must be something significantly different about my VS install with respect to 
this particular patch.  #26624 might or might not have something to do with it 
(see my posts today).  I can't tell from the logs which update the buildbots 
are running.

The fourth, x86windows7 crashed before running anything 
(http://buildbot.python.org/all/builders/x86%20Windows7%203.x/builds/11283/steps/test/logs/stdio),
 after having previously run part way through the tests.  It *might* have a 
similar problem, or might not.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26765] Factor out common bytes and bytearray implementation

[Terry J. Reedy]

Terry J. Reedy added the comment:

Serhiy, compiling 3.6 on Windows after this patch lets it start, but causes it 
to crash both importing tkinter and running patchcheck.  Compiling 3.6 after 
your previous patch for #27007 and the test suite runs without error.

--
nosy: +terry.reedy

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26765] Factor out common bytes and bytearray implementation

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26765] Factor out common bytes and bytearray implementation

[Roundup Robot]

Roundup Robot added the comment:

New changeset b0087e17cd5e by Serhiy Storchaka in branch 'default':
Issue #26765: Moved wrappers for bytes and bytearray methods to common header
https://hg.python.org/cpython/rev/b0087e17cd5e

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26765] Factor out common bytes and bytearray implementation

[Roundup Robot]

Roundup Robot added the comment:

New changeset c6c1882ecc77 by Serhiy Storchaka in branch 'default':
Issue #26765: Ensure that bytes- and unicode-specific stringlib files are used
https://hg.python.org/cpython/rev/c6c1882ecc77

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26765] Factor out common bytes and bytearray implementation

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
nosy: +haypo

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26765] Factor out common bytes and bytearray implementation

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Stage 4 patch moves thin wrappers around functions from bytes_methods.c to 
Objects/stringlib/transmogrify.h. The _method_ suffix is added to some names to 
distinguish them from existing stringlib names.

I'm not sure this is a good place. It may be better to add separate file (I'm 
going to move more simple wrappers here), but I don't have good name for it 
(bytes_methods.h is already used). Any ideas?

--
Added file: http://bugs.python.org/file42730/bytes_methods_stage4.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26765] Factor out common bytes and bytearray implementation

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Thank you Meador.

Stage 3 patch is simple. It just adds guards in bytes-specific and 
unicode-specific template files to be sure that they are not used with wrong 
type.

--
Added file: http://bugs.python.org/file42729/bytes_methods_stage3.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26765] Factor out common bytes and bytearray implementation

[Roundup Robot]

Roundup Robot added the comment:

New changeset f4406d746d27 by Serhiy Storchaka in branch 'default':
Issue #26765: Moved common code for the replace() method of bytes and bytearray
https://hg.python.org/cpython/rev/f4406d746d27

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26765] Factor out common bytes and bytearray implementation

[Meador Inge]

Meador Inge added the comment:

LGTM.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26765] Factor out common bytes and bytearray implementation

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


Added file: http://bugs.python.org/file42724/bytes_methods_stage2.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26765] Factor out common bytes and bytearray implementation

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Here is the second part of the patch. It moves common code for replace() method 
to template file Objects/stringlib/transmogrify.h.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26765] Factor out common bytes and bytearray implementation

[Roundup Robot]

Roundup Robot added the comment:

New changeset 41969033eb9d by Serhiy Storchaka in branch 'default':
Issue #26765: Moved common code and docstrings for bytes and bytearray methods
https://hg.python.org/cpython/rev/41969033eb9d

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26765] Factor out common bytes and bytearray implementation

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

>   1. The diffs are fairly large.  It might be easier to follow if you stage
>  the two steps above as separate commits (if possible).

OK. Here is the first part of the patch. It moves common code and docstrings in 
bytes_methods.c.

>   2. Why are some of the namings in stringlib inconsistent?  For example,
>  `stringlib_method_find` and `stringlib_index`.  In other words, why
>  do some names have the *_method_* part?

There were conflicts with existing names stringlib_find and stringlib_rfind.

--
Added file: http://bugs.python.org/file42502/bytes_methods_stage1.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26765] Factor out common bytes and bytearray implementation

[Meador Inge]

Meador Inge added the comment:

If I follow this correctly, then it seems like there are two
main pieces to this patch:

  1. Consolidating the following methods into `bytes_methods.c`:

 a. {bytes, bytearray}_find_internal

 b. {bytes, bytearray}_find

 c. {bytes, bytearray}_count

 d. {bytes, bytearray}_index

 e. {bytes, bytearray}_rfind

 f. {bytes, bytearray}_rindex

 g. {bytes, bytearray}_contains

 h. (_bytes, _bytearray}_tailmatch

 i. {bytes, bytearray}_startswith

 j. {bytes, bytearray}_endswith

  2. Consolidating the redundant implementations of the following functions
 into the stringlib:

 a. return_self

 b. countchar

 c. replace_interleave

 d. replace_delete_single_character

 e. replace_delete_substring

 f. replace_single_character_in_place

 g. replace_substring_in_place

 h. replace_single_character

 i. replace_substring

 j. replace

If so, then that seems reasonable to me and a nice cleanup.

A few comments:

  1. The diffs are fairly large.  It might be easier to follow if you stage
 the two steps above as separate commits (if possible).

  2. Why are some of the namings in stringlib inconsistent?  For example,
 `stringlib_method_find` and `stringlib_index`.  In other words, why
 do some names have the *_method_* part?

--
nosy: +meador.inge
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26765] Factor out common bytes and bytearray implementation

[Serhiy Storchaka]

New submission from Serhiy Storchaka:

Proposed patch factors out the implementation of bytes and bytearray by moving 
common code in separate files. This is not new approach, the part of the code 
is already shared.

The patch decreases the size of the source code by 862 lines.

--
components: Interpreter Core
files: bytes_methods.patch
keywords: patch
messages: 263458
nosy: serhiy.storchaka
priority: normal
severity: normal
status: open
title: Factor out common bytes and bytearray implementation
type: enhancement
versions: Python 3.6
Added file: http://bugs.python.org/file42465/bytes_methods.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com