Skip to content

Move all named constants to cdef?#42

Closed
mgeier wants to merge 1 commit intomasterfrom
cdef
Closed

Move all named constants to cdef?#42
mgeier wants to merge 1 commit intomasterfrom
cdef

Conversation

@mgeier
Copy link
Contributor

@mgeier mgeier commented May 29, 2014

I'm not quite sure if this is a good idea, any comments?

This is basically the logical continuation of #39, based on a discussion in #31.

@bastibe
Copy link
Owner

bastibe commented Jun 6, 2014

As much as I would like to move those constants back to C where they belong, I think as long as we have to build up the four dictionaries _formats, _subtypes, _endians, _str_types anyway, it is cleaner to just define the constants in there.

However, this got me thinking: Maybe we could cdef the complete portaudio header, without copying it into the Python file. This would remove that whole giant string from pysoundfile.py, and give us access to all the constants in there as well. Since we already link against the portaudio library, the header should be available anyway. This would make the programmatic extraction of those constants into dictionaries seem much less crazy. What do you think?

@mgeier
Copy link
Contributor Author

mgeier commented Jun 10, 2014

I think it's definitely a long-term goal to load the original header file instead of copying parts of it into the Python code.
However, there are a few constructs that CFFI doesn't support yet.
In case of sndfile.h this is the first error (but there are for sure more along the way):

CDefError: cannot parse "#ifndef SNDFILE_H"
:8: Directives not supported yet

I agree that the code for generating the dictionaries is not very nice.
I think we can drop this issue for now and re-open it once CFFI supports reading the whole header file.

@bastibe
Copy link
Owner

bastibe commented Jun 10, 2014

I think we can drop this issue for now and re-open it once CFFI supports reading the whole header file.

That seems like a good idea. If CFFI won't do it, maybe we can preprocess the header or ship an abbreviated version or something.

@mgeier
Copy link
Contributor Author

mgeier commented Jun 11, 2014

OK, I close it for now, we can dig it up at a later time if we feel like it ...

@mgeier mgeier closed this Jun 11, 2014
@mgeier mgeier deleted the cdef branch November 25, 2014 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants