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

wip docs and edge cases #15

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

wip docs and edge cases #15

wants to merge 7 commits into from

Conversation

ericphanson
Copy link
Member

@ericphanson ericphanson commented Dec 14, 2020

Comment on lines +12 to +15
function __init__()
gr()
GR.inline("png")
end
Copy link
Member Author

Choose a reason for hiding this comment

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

I think this might be better but I am not totally sure

Copy link
Member Author

Choose a reason for hiding this comment

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

@hannahilea pointed out that this code would be unnecessary after #1

src/utilities.jl Outdated
@@ -17,6 +17,7 @@ using the trapezoidal rule.
"""
function area_under_curve(x, y)
@assert length(x) == length(y)
length(x) == 0 && return 0.0
Copy link
Member Author

Choose a reason for hiding this comment

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

hit some strange edge case in beacon-biosignals/LighthouseFlux.jl#20 which is why I added this; possibly this should error or something instead though

Copy link
Contributor

Choose a reason for hiding this comment

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

In any case, let's add a unit tests that demonstrates the expected behavior.

@@ -0,0 +1,28 @@
# Evaluation metrics
Copy link
Member Author

Choose a reason for hiding this comment

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

I wonder if these should be docstrings? The reason I didn't do so from the start is that I meant this page to be more pedagogical (and hopefully to have references), instead of just documenting the programmatic API.

@ericphanson
Copy link
Member Author

ericphanson commented Dec 15, 2020

I added push previews and the cleanup workflow. This has been working well over at KeywordSearch and since this is a docs PR, I think it would help to see the generated docs!

Preview docs are here: https://beacon-biosignals.github.io/Lighthouse.jl/previews/PR15/

@ericphanson
Copy link
Member Author

Update on this:

I think #18 already addressed the indentation stuff from this PR and the GR stuff is removed in (#28), and the doc-cleanup script I included here is out-dated (newer one is at https://juliadocs.github.io/Documenter.jl/stable/man/hosting/#gh-pages-Branch).

So what's left that's useful here is the WIP terminology and the edge-case bit. Hopefully I'll be able to get back to those at some point and update this then.

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