Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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] add frequency API and compute coffeine API #51
[WIP] add frequency API and compute coffeine API #51
Changes from 4 commits
22b70a4
4ede86f
29396e6
3e5e887
bdf4329
84bd372
35c3cfd
3aa9158
4e69b6f
123e0dd
dacb831
d7ff4e6
b9a0a20
70dd8fb
ec958c6
ff34c4b
0f06397
7af2005
10d03fc
d21bfa1
242146b
655671e
8ca92f3
33e4e82
4182b86
fe0473e
ba21a31
0a1224a
4b40c08
8739790
400ae9e
babd18b
fcfc274
611d612
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
@apmellot @antoinecollas @apmellot @DavidSabbagh @hubertjb @bmalezieux
A small but important & overdue function :) What do you think about its naming? I think I am in principle happy with it. Other intuitive options would be to call it
make_coffeine_data_frame
ormake_covariance_data_frame
. I would be OK with making it explicit (data_frame
rather thandf
). Slight preference for coffeinemake_coffeine_data_frame
for branding and energizing weirdness. I understandmake_coffeine_data_frame
could be more boring and predictable (in the dispassionate scientific sense), which would be not bad either.Any thoughts? (for the bigger picture, please take minute to zoom out and look at the entire PR / the linked issues).
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.
I prefer
make_coffeine_data_frame
to be consistent with the functioncompute_coffeine
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.
well
compute_coffeine
is of course also up for discussion but I feel good about it actually.I can live with
make_coffeine_data_frame
andmake_covariance_data_frame
– although I have slight preference for coffeine as what we support here is not necessarily a covariance but any SPD matrix.