-
Notifications
You must be signed in to change notification settings - Fork 0
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
Implement helper function to process artificial star tests #37
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Been copy-pasting similar code around, figured it was worth making it nice and adding it to the package
Probably need to be broken out into extensions as I cant really figure out how to get this one function to work on both dataframes.jl and typedtables.jl. Just write it twice I guess...
Use package extensions to support both dataframes and typedtables. This works, but ugly code duplication. Figured out a different way to support both interfaces that I will use in main branch. This was useful to figure out how package extensions work though.
Stylistically preferred when you don't intend to extend `x`.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #37 +/- ##
==========================================
+ Coverage 77.19% 78.17% +0.98%
==========================================
Files 17 19 +2
Lines 1298 1361 +63
==========================================
+ Hits 1002 1064 +62
- Misses 296 297 +1 ☔ View full report in Codecov by Sentry. |
I think it is more correct to make the error and bias `NaN` when the completeness in the bin is 0. Clarified this in tests and wrote a covering case.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Been copy-pasting some code around for a while that calculates binned statistics on artificial star tests. This PR implements the
process_ASTs
method that accomplishes this.I wanted to support
DataFrames.DataFrame
andTypedTables.Table
but couldn't figure out a single implementation that would work on both. They iterate differently and I couldn't figure out a common mechanism to share the code.I decided to have separate implementations using package extensions. There is no default implementation for this method so it requires users to load one of the above packages before using it. I don't like how much code reuse there is between the two versions but right now I don't have a solution.
As this uses package extensions, the method requires Julia 1.9 or greater. I could look at some backporting methods like
https://github.com/cjdoris/PackageExtensionCompat.jl
but right now I don't care and am just not running the method's tests on older Julia versions.