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

Comments on documentation #36

Open
cwitkowitz opened this issue Nov 22, 2022 · 2 comments
Open

Comments on documentation #36

cwitkowitz opened this issue Nov 22, 2022 · 2 comments
Assignees

Comments

@cwitkowitz
Copy link

In general, the provided documentation is comprehensive and sufficient to understand the API at a high-level.

However, as with the source code blocks (issue #29), the html documentation itself appears to be out of date. For instance, see the call signature of Sound.condition, which is missing parameters included in analysis.py, Sound.filter_noise, which no longer even exists in analysis.py, etc.

I am also wondering whether the parameters (:param) and returned values (:return:) are being displayed properly on the webpages.

Another major comment is that no explanation, reference, or citation is provided for many of the features and concepts employed throughout the framework. While this is probably unnecessary for concepts as ubiquitous as the FFT, other features such as envelope, time damping, and peak damping (to name a few) are not adequately explained in the documentation. This requires people who are not immediately familiar with these terms and features (such as myself) to check the code to understand what is really happening.

While not critical to fix, here are some minor spelling/language issues (not exhaustive) that I encountered throughout the code and documentation:

  • “envelop” -> “envelope”
  • "associated to" -> “associated with”
  • “hemlotz” -> "Helmholtz" (keep case consistent)
  • “fourier” -> "Fourier" (keep case consistent)

The API Tutorial also seems to be slightly out of date, as it makes references to aspects of the framework which no longer seem to exist, such as the filtered_signal attribute of the Sound class or the timbre plotting feature of the Plot class.

Here are some typos (not exhaustive) I found in the API Tutorial:

  • “the are a” -> "this is"
  • “impacts” -> "impact"
  • “vary” -> "varies"
  • “fourier” -> "Fourier" (keep case consistent)
  • “treshold” -> "threshold"
  • “analysis” -> “analyses”
  • "cumilative" -> “cumulative”

Furthermore, there is some inconsistency across the API Tutorial with respect to capitalization and punctuation.

Also, is there any explanation for why the bass envelope droops prior to the onset in the plot derived from the call to Sound.plot_freq_bins in the API tutorial?

@olivecha
Copy link
Owner

olivecha commented Feb 1, 2023

olivecha added a commit that referenced this issue Feb 1, 2023
@olivecha olivecha self-assigned this Feb 1, 2023
olivecha added a commit that referenced this issue Feb 2, 2023
olivecha added a commit that referenced this issue Feb 2, 2023
@endolith
Copy link

Also:

  1. The readme uses guitarsound in some places and guitarsounds in others
  2. Probably should conda activate before pip install? I think that's necessary for conda to track pip-installed packages?
  3. Typically I want the readme to show a quick example of what the package does before going into the install instructions

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

3 participants