[issue14118] _pickle.c structure cleanup

[Martin v . Löwis]

Martin v. Löwis  added the comment:

Thanks for the effort, but I'm rejecting this now. I'm not fundamentally 
opposed to restructing this code, but I do think that your change would be a 
slight loss of readability. If you absolutely cannot stand working with such a 
large code, please try to find supporters of a change on python-dev.

Wrt. your actual approach: this is somewhat uncoommon. If you have many files, 
you'd rather expect them to be integrated in the build process (i.e. with 
Makefile and VS project file changes) rather than recursive includes.

--
resolution:  -> rejected
status: open -> closed

___
Python tracker 

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

[issue14118] _pickle.c structure cleanup

[Merlijn van Deen]

Merlijn van Deen  added the comment:

See https://bitbucket.org/valhallasw/cpython/src/ee0d2beaf6a4/Modules/_pickle.c 
for a rough structure overview - which maybe also explains why I thought 
restructuring made sense in the first place.

However, I'm not the person who has to maintain the module. If you're not 
interested, I'll just split out the module until I feel comfortable editing 
stuff, make my patch for #6784 and backport it to the 6500-line version.

--

___
Python tracker 

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

[issue14118] _pickle.c structure cleanup

[Martin v . Löwis]

Martin v. Löwis  added the comment:

I'm -1 on splitting the file. This is C, splitting it up will make it *harder* 
to understand, as you have to search across multiple files to find anything.

If you want to make it more readable, I propose that you
a) put a comment in the top explaining how the file is structured
b) put

/*/

  section headers between logical groups of functions.

--
nosy: +loewis

___
Python tracker 

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

[issue14118] _pickle.c structure cleanup

[Merlijn van Deen]

Merlijn van Deen  added the comment:

That makes sense. The goal was not so much cleaning up the module per se; 
rather, it was a result of trying to understand the general structure of 
_pickler.c specifically.

However, is there an intermediate level of 'modularization' you would propose? 
The idea is of course not to have 30 files open to make a small change, but 
rather have one or two reasonably small files open. My current approach was 
fine-grained on purpose - it's not that much work to recombine files.

I think it makes sense to split the PicklerObject into several parts:
  * the Pickler object (initialization/type/etc, maybe functions split off)
  * the actualy pickler implementation (dump/save/save_*) (but maybe in less 
files - see below)
  * the PicklerMemoProxy

for the picker implementation - specifically the picklers directory - my 
approach was to have a 'mirrored' directory for PicklerObject and 
UnPicklerObject: the methods to pickle and unpickle will be in the same files 
in the two directories (i.e. picklers/unicode.c will pickle str, 
unpicklers/unicode.c will unpickle them).

--

___
Python tracker 

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

[issue14118] _pickle.c structure cleanup

[Antoine Pitrou]

Antoine Pitrou  added the comment:

I think this is going overboard. _pickle.c is long but it defines two classes 
which are closely related to each other. I don't really get the point of 
exploding it into a myriad of 30-line files, especially if it means I now have 
to keep all these tiny files open when I want to change something in the pickle 
module.

If you want to find a target for modularizing, I think posixmodule.c would be a 
far better candidate.

--
nosy: +alexandre.vassalotti, pitrou
priority: normal -> low
versions:  -Python 3.4

___
Python tracker 

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

[issue14118] _pickle.c structure cleanup

[Merlijn van Deen]

New submission from Merlijn van Deen :

While working on #6784, I've looked at _pickle.c and found it quite... 
daunting: 6500 lines and 185kB. I have been working on a bit of cleanup, and 
I'd like some comments on this.

I'm working on adapting
_pickle.c into the following structure:

_pickle.c takes care of the module initialization
_pickle/*.c are helper files for this (global functions/definitions)
_pickle/PicklerObject contains all files that relate to the Pickler
class - initialization, all functions, etc, and
_pickle/PicklerObject/picklers/*.c contains all pickling functions,
split out into groups (corresponding to pickletools.StackObjects)

This should end in a tree where every .c module lists the relevant dependencies 
(and as such should be compilable on itself).

Currently, I'm at the point where PicklerObject roughly has the structure I 
want (although not every file is compilable on itself yet). As such, I feel 
this is the right moment to ask if it would be seen as an useful improvement in 
trunk, and to see if anyone has suggestions for improvements.

hg repos: 
https://bitbucket.org/valhallasw/cpython/src/0810ffadffa3/Modules/_pickle/PicklerObject
 (_pickle_clean branch)

--
components: Extension Modules
hgrepos: 114
messages: 154165
nosy: valhallasw
priority: normal
severity: normal
status: open
title: _pickle.c structure cleanup
type: enhancement
versions: Python 3.3, Python 3.4

___
Python tracker 

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