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

Documentations #236

Merged
merged 12 commits into from Oct 6, 2022
Merged

Documentations #236

merged 12 commits into from Oct 6, 2022

Conversation

Thanatoz-1
Copy link
Contributor

@Thanatoz-1 Thanatoz-1 commented Sep 27, 2022

[Draft]

This MR adds docs and their respective web objects in descending order.
Docs completed so far are:

  • Trim
  • Time stretch
  • Time Mask
  • Tanh Distortion
  • Shift
  • Seven band parametric eq
  • Room simulator
  • Reverse
  • Resample
  • Polarity inversion
  • Pitch Shift
  • Peaking filter
  • Padding
  • Normalize
  • mp3 compression
  • Low shelf filter
  • Low pass filter
  • Loudness normalization
  • Limiter
  • Lambda
  • High shelf filter
  • High pass filter
  • Gain
  • Gain transition
  • Clipping distortion
  • Clip
  • Band stop filter
  • Band pass filter
  • Apply impulse response
  • Air absoption
  • Add short noises
  • Add gaussian SNR
  • Add gaussian noise
  • Add background noise

@Thanatoz-1
Copy link
Contributor Author

Hey @iver56 ,
I am pushing my first documentation [Trim] here. I am considering a document to be complete only if it contains the waveform & spectrogram comparison, and audio samples. Please correct me wherever I might be wrong or else I will continue.

@Thanatoz-1 Thanatoz-1 changed the title [Draft] Documentations Documentations Sep 27, 2022
Copy link
Owner

@iver56 iver56 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks :) Can you also include your changes in generate_examples_for_doc.py?

docs/waveform_transforms/trim.md Outdated Show resolved Hide resolved
docs/waveform_transforms/trim.md Outdated Show resolved Hide resolved
@Thanatoz-1
Copy link
Contributor Author

Certainly, I'll also add changes in the generate_examples_for_doc.py

Thanatoz-1 and others added 2 commits September 27, 2022 19:46
Typo correction

Co-authored-by: Iver Jordal <1470603+iver56@users.noreply.github.com>
Adding unit

Co-authored-by: Iver Jordal <1470603+iver56@users.noreply.github.com>
@iver56
Copy link
Owner

iver56 commented Sep 27, 2022

I just merged some of my own documentation updates btw: #237

@Thanatoz-1
Copy link
Contributor Author

Thanks, Taking a pull now and updating the MR too.

@iver56
Copy link
Owner

iver56 commented Oct 5, 2022

I will wait for your changes in generate_examples_for_doc.py, and then I think this is ready to be merged

@iver56
Copy link
Owner

iver56 commented Oct 5, 2022

One thing I'm not sure if I like is the dependency on AddGaussianNoise_input.flac

There are pros and cons

Pro:

  • We save space in the repository and maybe save a little bit of network bandwidth for users that browse several documentation pages (because cache)

Con:

  • Documentation maintenance becomes harder. For example, if I update the audio example in the AddGaussianNoise documentation, other pages that rely on AddGaussianNoise_input.flac will be broken, and it can be hard to discover the error

What do you think? Does the pro outweigh the con?

@Thanatoz-1
Copy link
Contributor Author

Hey @iver56,

Sorry for not being able to reply sooner. I tried to think over this trade-off and for now, I feel let's try to avoid cons and stick to two separate files for each transform (input and output). I've modified the generate_examples_for_docs.py and have added most of the transforms for now. I'll push all the changes in some time and then you can merge.

@iver56
Copy link
Owner

iver56 commented Oct 6, 2022

Thanks 👍

@iver56 iver56 marked this pull request as ready for review October 6, 2022 07:10
@iver56 iver56 merged commit c680f16 into iver56:master Oct 6, 2022
@iver56
Copy link
Owner

iver56 commented Oct 6, 2022

I took the liberty of doing some minor improvements, I hope you don't mind: #240

It has been deployed to the documentation website now.

I also detected a minor alignment oddity in the Trim figure. It's basically my fault because I didn't think of preparing the plotting script for that case. I added an issue for that here: #239

@Thanatoz-1
Copy link
Contributor Author

Thanks a lot for merging @iver56. Please feel free to make any changes.

@Thanatoz-1
Copy link
Contributor Author

Also, for the remaining documents, I will be creating another MR and update the rest of it asap.

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

Successfully merging this pull request may close these issues.

None yet

2 participants