-
Notifications
You must be signed in to change notification settings - Fork 55
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
Docs: add example #96
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine in general I suppose, but a problem with examples is they're often not kept up to date when the rest of a repository is updated.
I agree with you. But the basic structure should not change enough to break a simple example like this one. |
This one is really old - but I think it is ok to merge. |
Sorry for the multi-year delay on this one, @heitorPB, and thanks for sticking with it! |
Ts = 1 / (1.1 * N) | ||
# Sample rate | ||
fs = 1 / Ts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
together, this is equivalent to saying that the sample rate is 1.1 * N
Hz, i.e. 4504.5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, I started a review hours ago and forgot to push submit
time_domain = plot(t, signal, title="Signal", xlims=(0, 4 / 60), xlabel="time (s)", label="") | ||
freq_domain = plot(freqs, abs.(F), title="Spectrum", xlims=(0, 200), xlabel="frequency (Hz)", label="") | ||
plot(time_domain, freq_domain, layout = 2) | ||
savefig("Wave.pdf") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could consider make this a Documenter @example
block, which will render the output into the document.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Btw, you don't have to make this change as part of this PR unless you want to. I mention it more as a note.)
Co-authored-by: Alex Arslan <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #96 +/- ##
=========================================
Coverage ? 73.45%
=========================================
Files ? 5
Lines ? 535
Branches ? 0
=========================================
Hits ? 393
Misses ? 142
Partials ? 0 ☔ View full report in Codecov by Sentry. |
Co-authored-by: Alex Arslan <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for the contribution, @heitorPB!
No description provided.