Core

[rbracco]

-Rewrite AudioToSpec and helper functions to be more flexible by adding a SpectrogramTransformer function that builds the appropriate pipeline in advance. -Fix the signature of AudioToSpec to be dynamic. -Update to fix breaking changes in fastai2.

[review-notebook-app[bot]]

Check out this pull request on 

You'll be able to see Jupyter notebook diff and discuss changes. Powered by ReviewNB.

[mogwai]

Looking good. Lets get this merged asap.

Few things:

• Can we remove the export at the bottom
• The augment notebook is 17mb! Any way we can reduce that, was struggling to load that over a bad connection
• I think that we should use a smaller data set for these notebooks. I recommend this

Also note sure we should have commented code like in section 4.2 in the augment notebook. We can always checkout previous commits to retrieve it.

[mogwai]

Realised that the notebooks don't have cleared outputs which is why core is 17mb. Would be cool if there was a way to clear outputs containing audio only as it is useful to see other outputs without running.

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

scart97 commented on 2020-02-17T00:03:27Z ----------------------------------------------------------------

Under SpectrogramTransformer, the dict(locals())solution just to get a dict with the function arguments seems overkill, as it returns a dict with the entire local symbol table. Changing it totransforms = _get_transform_list(mel=mel, to_db=to_db) and def _get_transform_list(**sg_type):will probably work fine, need to test.

Also the ordering of the function definition is confusing, I had to keep going up and down to follow the code. Just put the SpectrogramTransformer at the top of the cell and then the other functions following the order that they are called.

review-notebook-app[bot] commented 4 years ago

View / edit / reply to this conversation on ReviewNB

scart97 commented on 2020-02-17T00:03:28Z ----------------------------------------------------------------

Merge conflict not resolved

review-notebook-app[bot] commented 4 years ago

View / edit / reply to this conversation on ReviewNB

scart97 commented on 2020-02-17T00:03:28Z ----------------------------------------------------------------

Merge conflict

• © Githubissues.
• Githubissues is a development platform for aggregating issues.

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

scart97 commented on 2020-02-17T00:03:28Z ----------------------------------------------------------------

Merge conflict not resolved

review-notebook-app[bot] commented 4 years ago

View / edit / reply to this conversation on ReviewNB

scart97 commented on 2020-02-17T00:03:28Z ----------------------------------------------------------------

Merge conflict

• © Githubissues.
• Githubissues is a development platform for aggregating issues.

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

scart97 commented on 2020-02-17T00:03:28Z ----------------------------------------------------------------

Merge conflict

• © Githubissues.
• Githubissues is a development platform for aggregating issues.