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

allow figsize pass to audio_spectrogram_image #35

Closed
wants to merge 3 commits into from

Conversation

lyramakesmusic
Copy link

spectrogram_image allows passing an optional figsize, but that arg is not propagated to the audio_spectrogram_image wrapper. This edit lets you control that directly, for example if you want wider spectrograms: wandb.Image(audio_spectrogram_image(fakes, figsize=(15, 4)))

@drscotthawley
Copy link
Owner

Oooo, thanks for the fix, Lyra! But the checks failed because we need the change to go in the .ipynb file, not the .py file (as those are always overwritten by nbdev)
Guess I need to clarify that in the Contributing section of the README.
If you want to make the change yourself and re-submit, I'll gladly accept it so you get the PR credit!
Or if you'd rather I just make the same change myself in the .ipynb flie, I can do that.
Let me know what you want to do.

drscotthawley added a commit that referenced this pull request Jan 5, 2024
Clarified Contributing section, as per #35
@drscotthawley drscotthawley mentioned this pull request Jan 5, 2024
@lyramakesmusic
Copy link
Author

just to clarify- I do have to run nbdev, I can't just edit the ipynb and push? I've been editing via github web, not a local copy

@drscotthawley
Copy link
Owner

Yea you can't just edit on the web, the CI will fail due to timestamp errors, and the docs won't build correctly either.

As per Contributing:

  1. After editing notebooks, run nbdev_prepare

You need to run nbdev_prepare or at least nbdev_export, otherwise the .ipynb and .py files won't be sync'd and the Github Actions CI will throw an error and fail. But _prepare also builds the documentation.

@drscotthawley
Copy link
Owner

drscotthawley commented Jan 5, 2024

It might be easier if I just make the change and push it ,since I'm used to this stuff. (I could do it, and add a note in the Comment that I'm pasting in your change.)

But I'd still be happy to wait for you to do it if you want to.

@lyramakesmusic
Copy link
Author

works for me! I can try out the build tools later if I run into something again

@drscotthawley
Copy link
Owner

Ok, working on it!
Having trouble with nbdev_prepare myself right now, crashing. :-/ . Might have to apply the change on Linux instead of my Mac....

@drscotthawley
Copy link
Owner

Closing this PR, as now implemented via Commit 448a754! 🥳

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.

2 participants