-
Notifications
You must be signed in to change notification settings - Fork 2
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
138 fix the metric for dataless formations #142
Conversation
countDataless :: (MonadState Metrics m) => [Binding] -> m () | ||
countDataless bindings = do | ||
let countDeltas = count (\case DeltaBinding _ -> True; _ -> False) | ||
deltas = countDeltas (bindings <> concatMap (\case AlphaBinding _ (Formation bindings') -> bindings'; _ -> []) bindings) |
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 like a very bad case
-oneliner. Why not introduce a helper properly?
count :: (a -> Bool) -> [a] -> Int | ||
count x = length . filter x | ||
|
||
countDataless :: (MonadState Metrics m) => [Binding] -> m () |
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.
This function requires documentation and a doctest.
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.
added in 2fa396e
4926ee0
to
2fa396e
Compare
countDataless :: (Num a) => [Binding] -> a | ||
countDataless bindings = | ||
let countDeltas = count (\case DeltaBinding _ -> True; _ -> False) | ||
nestedBindings = concatMap (\case AlphaBinding _ (Formation bindings') -> bindings'; _ -> []) bindings |
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.
This one-line case
is still terrible :)
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 see no issues with it as long as I have only two cases, where the second one is trivial. Please, feel free to suggest changes.
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 think it makes sense to factor out this case
into a separate named helper function and use indentation, not ;
for the cases. This is not critical, just a style thing.
Closes #138
PR-Codex overview
This PR enhances the
eo-phi-normalizer
by improving metrics calculation and adding a new function for counting dataless formations.Detailed summary
dataless
metric values inmetrics.yaml
countDataless
to count dataless formations in bindingsCollect.hs
to utilize the new function