Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-order functions in pysoundfile.py #57

Closed
mgeier opened this issue Jul 15, 2014 · 6 comments
Closed

Re-order functions in pysoundfile.py #57

mgeier opened this issue Jul 15, 2014 · 6 comments
Milestone

Comments

@mgeier
Copy link
Contributor

mgeier commented Jul 15, 2014

I think it's better if the high-level functions are listed earlier in the source file (and therefore in the Sphinx documentation, see also #27).

I think the read() function should be the very first, followed by the write() function.

Then probably available_formats(), available_subtypes(), default_subtype() and format_check().

After that, probably the blocks() function.

I guess it makes sense to put all the underscore-prefixed functions right after that.
Or should we put them immediately before or after the function they are used in?

I would move the class definition of Soundfile to the very end.
Within the class we should probably also move some methods around, but I'm not sure what's the best strategy there.

We should do this before re-writing the docstrings in NumPy style (#27), but probably after setting up Sphinx.

@mgeier
Copy link
Contributor Author

mgeier commented Jul 15, 2014

BTW, the module docstring should move to the very top of the file.
The version number should be defined before the imports.

@bastibe
Copy link
Owner

bastibe commented Jul 17, 2014

I think that the order of things in pysoundfile.py should be the order in which the file reads best.

The biggest issue with that are the CFFI declarations at the start of the file (the long string and the dictionaries). We could move those into their own file and import them from there.

After that, SoundFile is really required knowledge in order to understand the functions. However, while this reads well if you read the source, it is not that important for the documentation. I would therefore propose to move SoundFile to a different file as well. I think that we have reached a level of complexity that really warrants this. This also means that most of the libsndfile helpers have to move to soundfile.py as well, which cleans the main file from most of the underscore methods.

However, SoundFile is still a very valuable API, and I still want to be able to from pysoundfile import SoundFile, so we'll probably have to have some lengthy from soundfile import SoundFile, available_formats, available_subtypes, default_subtype, format_check at the beginning of the file.

This would leave the main file pretty empty. But maybe that is not such a bad thing.

Any thoughts about this?

@mgeier
Copy link
Contributor Author

mgeier commented Aug 5, 2014

I'm not sure if splitting pysoundfile into several files is an improvement in the current state.
Having everything in one file also has its benefits.

The CFFI declarations are indeed annoying, but I'm hoping that some time in the future it will be possible to load a separate header file instead of copying the code. Then our code should become much cleaner.

After that, SoundFile is really required knowledge in order to understand the functions.

I'm not sure about that.
I guess it depends if you're trying to understand the thing in a bottom-up or a top-down way.
Both are valid viewpoints and the language itself allows both.

I stumbled upon a source file for CPython's open() function.
In there, first the high-level open() function is listed and only afterwards the classes which it needs (e.g. BufferedReader) and all their base classes are defined.

Sure, a single example isn't quite representative, but probably we'll find more examples.

But anyway, the order of the Sphinx documentation is much more important than the order in the source file(s).

@bastibe
Copy link
Owner

bastibe commented Aug 6, 2014

Ordering functions before classes is fine by me.

I'll experiment with splitting pysoundfile into several files a bit. We'll see how it feels.

@mgeier mgeier changed the title Re-order functions in pysoundfile.py? Re-order functions in pysoundfile.py Oct 22, 2014
mgeier added a commit that referenced this issue Oct 22, 2014
mgeier added a commit that referenced this issue Oct 22, 2014
@mgeier mgeier modified the milestone: 0.6.0 Oct 26, 2014
@mgeier
Copy link
Contributor Author

mgeier commented Nov 16, 2014

The order was changed in #74.
Now the free functions come first, then comes the SoundFile class, and finally the underscore-prefixed functions.
Within the class, there are first the "special" methods, then the "normal" methods and finally the underscore-prefixed methods.

Currently, I don't see a need for splitting the code into several modules.

I vote for closing this issue.

@bastibe
Copy link
Owner

bastibe commented Nov 17, 2014

I agree.

@bastibe bastibe closed this as completed Nov 17, 2014
mgeier added a commit that referenced this issue Dec 17, 2014
mgeier added a commit that referenced this issue Dec 17, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants