many changes: formats, modes, dtype, sf_open_fd(), ...

[mgeier]

This commit holds several changes which were discussed in #22 (which was
itself based on #18).

Change handling of file formats:
File formats are handled as three simple strings: format, subtype and endian.

Add "which" argument to seek().
This is needed because the combination with logical or (e.g. SEEK_SET | READ)
isn't possible with string formats.

Update frames counter on write().

Hide all non-API names in module namespace (by prefixing _).
This is relevant for tools that do introspection, e.g. for
auto-completion in IDEs.

Support sf_open_fd(), remove obsolete argument virtual_io
The first constructor argument can now be:

• a string (path to a file)
• an int (a file descriptor)
• a file-like object
  The argument virtual_io is not needed, because this can be detected
  automatically.
  Closes Implement sf_open_fd? #19

Get file extension from 'name' attribute of a file-like object.

Change public attributes of SoundFile class to properties.

Add 'name' property.

Add properties 'format_info' and 'subtype_info'.

Proper handling of dtype's in read() and write().

Add function default_subtype().

[bastibe]

(I'll put these comments in the pull request and not in the commit, so as to avoid losing them when the PR changes)

I have a few comments:

• In line 344 you are using line continuations instead of a multi-line string. Is there a specific reason for that?
• In the long term, I would like to change string formatting to use str.format instead of the % operator.
• I think we should rename vio to virtual_io. Reading vio might confuse users.
• Using lambdas in properties is nice, but in case anything goes wrong, the stack trace would contain an unnamed function. There doesn't seem to be any place that could throw exceptions in this case though, so it's all right.
• line 402 still contains one ffi instead of _ffi.
• why does _format_info take _GET_FORMAT_INFO as an argument? It seems that it does not have any purpose other than getting info anyway. Maybe this could be factored into an sf_command wrapper and a separate _format_info method. On a related note, I think that _GET_FORMAT_INFO should live in the _ffi.cdef instead of the Python code.

Only the last two points are immediately actionable. The rest of the PR looks great! If you get this ready today, I'll start writing tests tomorrow. During all this refactoring action, a test suite is becoming more and more valuable.

[mgeier]

• In line 344 you are using line continuations instead of a multi-line string. Is there a specific reason for that?

I don't want to put newlines into the error string, a multi-line string would do that.

But if I use the string literal directly as argument (without defining a variable), I can at least get rid of the line continuations.

Is that better?

• In the long term, I would like to change string formatting to use str.format instead of the % operator.

I don't have a strong opinion about that, I just used what was used in the rest of the code.

I want to point out, however, that the automatic numbering of format arguments is not supported in Python 2.6 (e.g. "{} {}".format(1, 2) doesn't work).

Anyway, I have a few more commits in the pipeline, I would prefer if we change that at a later time.

• I think we should rename vio to virtual_io. Reading vio might confuse users.

Sure, why not.
But again, I would like to do that at a later time, because it may mess up my next few commits.

• Using lambdas in properties is nice, but in case anything goes wrong, the stack trace would contain an unnamed function. There doesn't seem to be any place that could throw exceptions in this case though, so it's all right.

Yeah, I don't think that's a real problem.

And defining the functions with def and using the @property decorator would just need way too many lines!

• line 402 still contains one ffi instead of _ffi.

Nice catch.

• why does _format_info take _GET_FORMAT_INFO as an argument? It seems that it does not have any purpose other than getting info anyway.

Good question, the reason is in one of my next commits.
There it will be needed in the functions available_formats() and available_subtypes().

Maybe this could be factored into an sf_command wrapper and a separate _format_info method.

Probably.
Can we tackle this at a later point?

On a related note, I think that _GET_FORMAT_INFO should live in the _ffi.cdef instead of the Python code.

Can enums be used in CFFI?

If yes I'm wondering why we're not using them with the file formats and the other stuff as well.

Anyway, it would be nice if we could change that at a later time after my next few commits.

[bastibe]

Alright, let's postpone everything save the ffi typo. None of this is critical in any way.

On 20. April 2014 20:38:40 MESZ, Matthias Geier notifications@github.com wrote:

• In line 344 you are using line continuations instead of a
  multi-line string. Is there a specific reason for that?

I don't want to put newlines into the error string, a multi-line string
would do that.

But if I use the string literal directly as argument (without defining
a variable), I can at least get rid of the line continuations.

Is that better?

• In the long term, I would like to change string formatting to use
  str.format instead of the % operator.

I don't have a strong opinion about that, I just used what was used in
the rest of the code.

I want to point out, however, that the automatic numbering of format
arguments is not supported in Python 2.6 (e.g. "{} {}".format(1, 2)
doesn't work).

Anyway, I have a few more commits in the pipeline, I would prefer if we
change that at a later time.

• I think we should rename vio to virtual_io. Reading vio might
  confuse users.

Sure, why not.
But again, I would like to do that at a later time, because it may mess
up my next few commits.

• Using lambdas in properties is nice, but in case anything goes
  wrong, the stack trace would contain an unnamed function. There doesn't
  seem to be any place that could throw exceptions in this case though,
  so it's all right.

Yeah, I don't think that's a real problem.

And defining the functions with def and using the @property
decorator would just need way too many lines!

• line 402 still contains one ffi instead of _ffi.

Nice catch.

• why does _format_info take _GET_FORMAT_INFO as an argument? It
  seems that it does not have any purpose other than getting info anyway.

Good question, the reason is in one of my next commits.
There it will be needed in the functions available_formats() and
available_subtypes().

Maybe this could be factored into an sf_command wrapper and a
separate _format_info method.

Probably.
Can we tackle this at a later point?

On a related note, I think that _GET_FORMAT_INFO should live in the
_ffi.cdef instead of the Python code.

Can enums be used in CFFI?

If yes I'm wondering why we're not using them with the file formats and
the other stuff as well.

Anyway, it would be nice if we could change that at a later time after
my next few commits.

---

Reply to this email directly or view it on GitHub:
https://github.com/bastibe/PySoundFile/pull/31#issuecomment-40901590

Diese Nachricht wurde von meinem Mobiltelefon mit Kaiten Mail gesendet.

[bastibe]

But if I use the string literal directly as argument (without defining a variable), I can at least get rid of the line continuations.

Is that better?

Yes, much better! As a general rule, I try to avoid backslash-line-continuations wherever possible. This is as per pep8.
I want to point out, however, that the automatic numbering of format arguments is not supported in Python 2.6 (e.g. "{} {}".format(1, 2) doesn't work).

I would boldly say that we can drop 2.6 compatibility then. 2.7 and 3.3 is fine by me. What do you think? The main argument against percent-formatting is that it is deprecated, and that operator precedence can make it tricky.
Anyway, I have a few more commits in the pipeline, I would prefer if we change that at a later time.

Yes, by all means. I just wanted to put all of these in writing, so I won't forget them!
Can enums be used in CFFI?

Yes, they are treated as integers.
If yes I'm wondering why we're not using them with the file formats and the other stuff as well.

Initially, because we needed the Python variables anyway, so there was no point in duplicating them in both the C code and the Python code. I guess this argument still holds. Since we are not compiling against the actual libsndfile header, we'd have to copy all those definitions into the ffi.cdef block, and then repeat them in the dictionaries.

Anyway, I see that the ffi-bug is fixed, so just merge when you are ready!

[mgeier]

I want to point out, however, that the automatic numbering of format arguments is not supported in Python 2.6 (e.g. "{} {}".format(1, 2) doesn't work).

I would boldly say that we can drop 2.6 compatibility then. 2.7 and 3.3 is fine by me. What do you think?

I personally don't need 2.6 compatibility (I don't even need 2.7). But I think if CFFI supports it (which it does) and if it's not too much additional work for us, we should keep it anyway.

I didn't try PySoundFile on Python 2.6 because I had some problems with setuptools, so I don't even know if there's something else that breaks compatibility.

Did you try it?

The main argument against percent-formatting is that it is deprecated, and that operator precedence can make it tricky.

Is it really formally deprecated?
Many people claim that, but I didn't find a reliable source.
In the current documentation (Python 3.4) there is no mention of deprecation: https://docs.python.org/3/library/stdtypes.html#old-string-formatting

I agree that the new format() method is much superior (and I use it all the time), but we're using the %-formatting only in very few very simple cases and I don't think it's worth dropping 2.6 compatibility just for those few (and unproblematic) occurrences.

[bastibe]

I'll implement a test suite later today. Then, we'll have a better idea about what works and what doesn't.

As for string formatting, you are correct. It is not deprecated. However, the docs claim that percent-formatting is error-prone:

The formatting operations described here exhibit a variety of quirks that lead to a number of common errors (such as failing to display tuples and dictionaries correctly). Using the newer str.format() interface helps avoid these errors, and also provides a generally more powerful, flexible and extensible approach to formatting text.

Also, a useful discussion on SO: http://stackoverflow.com/questions/5082452/python-string-formatting-vs-format

[mgeier]

I'll implement a test suite later today. Then, we'll have a better idea about what works and what doesn't.

Great, I'm looking forward to that!

About % string formatting: I checked the current PySoundFile code, and 6 out of in total 8 occurrences are of this form:

"some text %s some text" % repr(some_variable)

I think this very specific usage is perfectly safe and not error-prone. repr() always returns a single string, so the tuple problem goes away.

The only still unsafe spot is in __setattr__(), where repr() should be added.
In __setitem__() there is another occurence without repr(), but there the arguments are already in a tuple, so this should be safe, too (and probably I can convince you some day to finally get rid of this method anyway).

Again, I do think .format() is generally better, I just don't want to loose 2.6 compatibility if it's only because of this minor inconvenience.