Single file restriction

[Pavel Zlatovratsky]

PyPNG have option to distributed as single file. That is nice option to embeding, but...
Why this also lead to restriction development into single file?
https://github.com/drj11/pypng/pull/21#issuecomment-25708701

[David Jones]

It doesn't lead to that restriction. The idea that the primary file
for editing should be the single file png.py is a separate thing.

I don't like the idea that a .py file should be compiled from other files.

Can you describe what you're trying to achieve and why you think
separating png.py into separate files helps with that?

The previous Cython changes put the Python code in png.py (see
https://github.com/drj11/pypng/commit/db9f958bc41db904b3d2c921b964b4e03d5ee018
). Why do you find that so objectionable?

David Jones

[Pavel Zlatovratsky]

I don't like the idea that a .py file should be compiled from other files.

I don't like this idea  too, but I made this as dirty hack for single-file mode. (This part of code shouldn't be used when pngfilters.py in folder)
 

Can you describe what you're trying to achieve and why you think
separating png.py into separate files helps with that?

Realy I think there are many things that will be good with switch from single-file module into package.
Exempli gratia:
1) Usage of iccp.py code. (it can be copied in png.py, but...)
2) separate different part of code into different files. Like separate file for pngsuite, may be unittests.. filters too of cource, may be some code for manipulation with alpha as separate module etc.

4K lines is very big module IMHO, compare to standard library where most modules are less than 2k lines or png plugin of PIL/Pillow (latest in git is 668 lines)

The previous Cython changes put the Python code in png.py (see
https://github.com/drj11/pypng/commit/db9f958bc41db904b3d2c921b964b4e03d5ee018
). Why do you find that so objectionable?

Because previous cython changes made separate cython code while I'm trying to compile with cython same code that used in pure-python mode.
So there are three ways:
1) use pngfilters.py that compiled in pngfilters.c (and later in *.pyd or *.so). If compilation good than compiled version override pngfilters.py, if it's broken than pngfilters.py used  - that was rejected because of single-file limitation

2) same as above, but pngfilters.py also embeded in png.py. Like:
pngfilters.c <= pngfilters.py => (part in png.py)
*if compiled found - compiled used
*if no compiled  - pngfilters.py used
*if no pngfilters.py - embeded part used
this was rejected too.

3) My last hope. pngfilters.py pulled, 'unimported' from png.py.
(part in png.py) => pngfilters.py => pngfilters.c
This one I work on right now.
This is much harder to implement, seems I have to change some conceptions about filters (but this also can improve speed more).

Main reason is Don't Repeat Yourself. In particular I want if any possible bug will be fixed in python there should be no cython code with this bug (and bugged module for cython-users)
 
P.S. Write in python is easier for me than write in English, so I show you first prototype with next answer, I hope.

[David Jones]

On 11 October 2013 07:11, Pavel Zlatovratsky

<pavelzla...@gmail.com> wrote:
>>
>> Can you describe what you're trying to achieve and why you think
>> separating png.py into separate files helps with that?
>
> Realy I think there are many things that will be good with switch from
> single-file module into package.
> Exempli gratia:
> 1) Usage of iccp.py code. (it can be copied in png.py, but...)

Most of the iccp.py code does not belong in a PNG package.

> 2) separate different part of code into different files. Like separate file
> for pngsuite, may be unittests.. filters too of cource, may be some code for
> manipulation with alpha as separate module etc.

The unittests are going to go in a separate file.

PngSuite is only copied into png.py for testing; it will be moved to a
separate file.

The filters will not be moved into a separate file.

> 4K lines is very big module IMHO, compare to standard library where most
> modules are less than 2k lines or png plugin of PIL/Pillow (latest in git is
> 668 lines)

png.py is a big file. That is true.

But if you're going to criticise that, then merely comparing it to
files in the standard library is not going to help. You have to argue
that there is some benefit to splitting it into smaller files! Note
that I already said the tests and PngSuite are going, so it will be
smaller anyway.

>> The previous Cython changes put the Python code in png.py (see
>>
>> https://github.com/drj11/pypng/commit/db9f958bc41db904b3d2c921b964b4e03d5ee018
>> ). Why do you find that so objectionable?
>
> Because previous cython changes made separate cython code while I'm trying
> to compile with cython same code that used in pure-python mode.

Ah. I think this is what I wanted you to say much much earlier.

You're trying to compile with Cython, and you want Cython to use
exactly the same code.

I like that as a goal. A lot.

I'm just not interested in Cython.

> So there are three ways:

[...cut...]

> 3) My last hope. pngfilters.py pulled, 'unimported' from png.py.
> (part in png.py) => pngfilters.py => pngfilters.c
> This one I work on right now.
> This is much harder to implement, seems I have to change some conceptions
> about filters (but this also can improve speed more).

This approach seems best to me. I'm not really sure what you mean by
"change some conceptions about filters", but I'm sure the code will
show me.

Why can't Cython just compile the whole of png.py? Why compile just the filters?

Cheers,
drj

[Pavel Zlatovratsky]

>You have to argue 
>that there is some benefit to splitting it into smaller files! Note 
>that I already said the tests and PngSuite are going, so it will be 

>smaller anyway. 

Of course none of reasons is not enough to switch into package, but all of them together enough.

If you have your ideas about most of them - than ok. Only filters is not enough reason, I agree.

>Ah. I think this is what I wanted you to say much much earlier. 

My fault.

That was my first commit. And I thought it was <authors> idea about cython, so few words I left would make whole idea obvious.

https://github.com/Scondo/pypng/commit/59a42c3a71449a51a82902c9551040f1530df409

Later I found that it was not your code, but I remember this feeling that switching to augmented code is obvius.

> I'm not really sure what you mean by "change some conceptions about filters",

I move filter functions to Filter object to get rid of static methods and whole concept 'simulate module with class'.

And keep playing with not exposing them from binary module.

>Why can't Cython just compile the whole of png.py? Why compile just the filters?

Because Cython become really effective only with static typing and without many of python nice features (like 'function is object') .

But in this case working code inside all wrappers is about 1:1 c-code.

Here is sample (part of 'filter_scanline' with function selection switched to 'if' cascade)

https://github.com/Scondo/pypng/blob/filter/code/pngfilters.c#L3868 

(prototype working but now is reealy bad, many code don't easily moved to cython and style should be cleaned too)

[Pavel Zlatovratsky]

I'm about finish my work on filters and cython.

https://github.com/Scondo/pypng/compare/filter

Two things left:
1) Use Cython compilation instead of distributing .c file  https://github.com/drj11/pypng/issues/23
Your were wrong according to cython guidelines http://docs.cython.org/src/reference/compilation.html#distributing-cython-modules
But 'unimport' and complex instructions about it give me a good reason to agree with you.

2)Rename all methods that are compiled as 'cdef' (i.e. unavailible from pyhon to provide faster 'c' code) to private (__do*, __undo*).
I'll add some remarks about his in source code comments.
To reduce chance that someone use these methods in pure-python mode and such code will be broken with cython.

It would be nice to hear your opinion about these things.