[issue12808] Coverage of codecs.py

[Mark Lawrence]

Change by Mark Lawrence :


--
nosy:  -BreamoreBoy

___
Python tracker 

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

[issue12808] Coverage of codecs.py

[Martin Panter]

Changes by Martin Panter vadmium...@gmail.com:


--
nosy: +vadmium

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

[issue12808] Coverage of codecs.py

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

See also issue20420.

--
nosy: +serhiy.storchaka

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

[issue12808] Coverage of codecs.py

[Walter Dörwald]

Walter Dörwald added the comment:

The requirement that getstate() returns a (buffer, int) tuple has to do with 
the fact that for text streams seek() and tell() somehow have to take the state 
of the codec into account. See 
_pyio.TextIOWrapper.(seek|tell|_pack_cookie|_unpack_cookie).

However I can't remember the exact history of the specification.

--
nosy: +doerwalter

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

[issue12808] Coverage of codecs.py

[Marc-Andre Lemburg]

Marc-Andre Lemburg added the comment:

The patch would have to be updated to 3.5 (part of it no longer applies), but 
other than that I think it's fine.

It may make sense to readd the comment for .getstate() to keep the state as 
simple as possible (The implementation should make sure that ``0`` is the most 
common state.), but without requiring a specific number, type, etc.

--

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

[issue12808] Coverage of codecs.py

[Mark Lawrence]

Mark Lawrence added the comment:

Would someone who's commented previously do a commit review please?

--
nosy: +BreamoreBoy

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

[issue12808] Coverage of codecs.py

[Terry J. Reedy]

Changes by Terry J. Reedy tjre...@udel.edu:


--
components: +Library (Lib), Tests
versions: +Python 3.3 -Python 3.4

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12808
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12808] Coverage of codecs.py

[Tennessee Leeuwenburg]

Tennessee Leeuwenburg tleeuwenb...@gmail.com added the comment:

Here is a stab at updated documentation. I would suggest that if further 
changes are recommended to the documentation, that a core committer go ahead 
and make them. I'm absolutely more than happy to keep taking stabs at it, but 
ultimately I probably don't understand the purpose of these classes as well as 
some of the rest of you, and I don't feel best placed to decide exactly how 
this should read

--
Added file: http://bugs.python.org/file23049/codecs.diff

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12808
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12808] Coverage of codecs.py

[Marc-Andre Lemburg]

Marc-Andre Lemburg m...@egenix.com added the comment:

Tennessee Leeuwenburg wrote:
 
 Tennessee Leeuwenburg tleeuwenb...@gmail.com added the comment:
 
 Thanks for the review. Here is a patch incorporating the two comments being 
 to move some comments.

Hmm, the documentation patch doesn't appear to have changed. Did you
upload the right patch ?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12808
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12808] Coverage of codecs.py

[STINNER Victor]

Changes by STINNER Victor victor.stin...@haypocalc.com:


--
nosy: +haypo

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12808
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12808] Coverage of codecs.py

[Marc-Andre Lemburg]

Marc-Andre Lemburg m...@egenix.com added the comment:

Nick Coghlan wrote:
 
 As a separate, but related point, IncrementalDecoder.getstate() includes an 
 explanation on how to save arbitrary state as an integer, but no such 
 explanation (not even a reference to the IncrementalDecoder version) is 
 present in the IncrementalEncoder.getstate() docs.
 
 Adding MAL, since I'd like an expert opinion. Is the API less stringent than 
 the docs state, or should BufferedIncrementalEncoder be fixed to always 
 return the state as an integer?
 
 [1] 
 http://docs.python.org/dev/library/codecs#codecs.IncrementalEncoder.getstate

I'm not sure how that description got into the docs. It must
have been added in the 3.x branch, since the 2.7 version of the
docs doesn't document those method at all.

FWIW: The .getstate() and .setstate() don't restrict the type of data
used for storing the state. The only requirement is that the data
returned by .getstate() can be passed to .setstate() in order to
recreate the internal state used by the codec:

def getstate(self):

Return the current state of the encoder.

return 0

def setstate(self, state):

Set the current state of the encoder. state must have been
returned by getstate().

For practical reasons, the used data should be pickleable.

The interface is very similar to the __getstate__/__setstate__
interface used by pickle.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12808
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12808] Coverage of codecs.py

[Tennessee Leeuwenburg]

Tennessee Leeuwenburg tleeuwenb...@gmail.com added the comment:

Some more tests, updated initial state of BufferedIncrementalEncoder to be the 
correct type, updated rst file. Bit tired, hope I got it right!

Thanks for the feedback everyone, helps me to get it done, even if it's more 
work for you...

--
Added file: http://bugs.python.org/file22996/codecs.diff

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12808
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12808] Coverage of codecs.py

[Marc-Andre Lemburg]

Marc-Andre Lemburg m...@egenix.com added the comment:

Tennessee Leeuwenburg wrote:
 
 Tennessee Leeuwenburg tleeuwenb...@gmail.com added the comment:
 
 Some more tests, updated initial state of BufferedIncrementalEncoder to be 
 the correct type, updated rst file. Bit tired, hope I got it right!
 
 Thanks for the feedback everyone, helps me to get it done, even if it's more 
 work for you...

I think you should simply drop this whole part:


The implementation should make sure that ``0`` is the most common state. (States
that are more complicated than integers can be converted into an integer by
marshaling/pickling the state and encoding the bytes of the resulting string
into an integer).


or, better, rephrase it so that it becomes clear that the codec
implementation may use any type of pickleable object to represent
the internal state. The only requirement is that .setstate()
has to be able to read back the state returned by .getstate().

We have codecs in the stdlib that return integers, bytes/string
and even tuples (see the io module and the UTF-16 codec as example),
so the documentation is clearly wrong.

Thanks,
-- 
Marc-Andre Lemburg
eGenix.com


2011-10-04: PyCon DE 2011, Leipzig, Germany43 days to go

::: Try our new mxODBC.Connect Python Database Interface for free ! 

   eGenix.com Software, Skills and Services GmbH  Pastor-Loeh-Str.48
D-40764 Langenfeld, Germany. CEO Dipl.-Math. Marc-Andre Lemburg
   Registered at Amtsgericht Duesseldorf: HRB 46611
   http://www.egenix.com/company/contact/

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12808
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12808] Coverage of codecs.py

[Tennessee Leeuwenburg]

Tennessee Leeuwenburg tleeuwenb...@gmail.com added the comment:

Thanks for the review. Here is a patch incorporating the two comments being to 
move some comments.

--
Added file: http://bugs.python.org/file23007/codecs.diff

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12808
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12808] Coverage of codecs.py

[Tennessee Leeuwenburg]

New submission from Tennessee Leeuwenburg tleeuwenb...@gmail.com:

Increases the coverage of codecs.py by about 3 lines...

--
files: mywork.patch
keywords: patch
messages: 142661
nosy: ncoghlan, tleeuwenburg
priority: normal
severity: normal
status: open
title: Coverage of codecs.py
versions: Python 3.4
Added file: http://bugs.python.org/file22983/mywork.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12808
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12808] Coverage of codecs.py

[Tennessee Leeuwenburg]

Tennessee Leeuwenburg tleeuwenb...@gmail.com added the comment:

Also some pep8 compliance outside of the scope of the added lines of code.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12808
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12808] Coverage of codecs.py

[Nick Coghlan]

Nick Coghlan ncogh...@gmail.com added the comment:

Tennessee, there were a few issues in the original patch - see the attached 
update (mostly the use of assert statements instead of methods and the 
overbroad scope of the context manager when checking for an expected exception).

However, in getting ready to commit this, I noticed something interesting. The 
docs for the IncrementalEncoder.getstate() API [1] state that the value 
returned *must* be an integer.

BufferedIncrementalEncoder doesn't obey that limitation - when data is 
buffered, it returns the raw buffered string instead of encoding it as some 
kind of integer.

As a separate, but related point, IncrementalDecoder.getstate() includes an 
explanation on how to save arbitrary state as an integer, but no such 
explanation (not even a reference to the IncrementalDecoder version) is present 
in the IncrementalEncoder.getstate() docs.

Adding MAL, since I'd like an expert opinion. Is the API less stringent than 
the docs state, or should BufferedIncrementalEncoder be fixed to always return 
the state as an integer?

[1] http://docs.python.org/dev/library/codecs#codecs.IncrementalEncoder.getstate

--
nosy: +lemburg
Added file: http://bugs.python.org/file22990/issue12808_codecs_coverage.diff

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12808
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com