6 kyu

Group-by and Sum

251 of 279Fbasham
Description
Loading description...
Fundamentals
View
AllIssues3QuestionsSuggestions1Show Resolved
  • Please sign in or sign up to leave a comment.
  • danganminh Avatar

    NICEEEEEEEEEEEE

  • tobeannouncd Avatar
  • brodiemark Avatar

    Hi Fbasham,

    Nice problem! Are you interested in translations?

    Regards,
    brodiemark

  • silviluliuma Avatar

    It is a good kata. However, in my opinion it is not a 6 kyu kata...

  • FArekkusu Avatar

    The grouping keys and aggregated values should be separated, e.g. [(keys_list_1, values_list_1), (keys_list_2, values_list_2), ...]. Although I think it'd be better to return a dictionary instead, e.g. {keys_tuple_1: values_list_1, keys_tuple_2: values_list_2, ...}.

    Also, my grouping/aggregating explanation has to be copied back into the description from the revisions page (and adjusted for the new requirements).

  • Blind4Basics Avatar

    Hi,

    your description is lacking some stuff. A beginner will never understand what you're actually asking for...

    • you're somewhat hidding the fact that the indexes are used to define the key identifying a group (that part, is still understandable)
    • you're completely hidding the fact that the aggregation has to be done on all the other indexes! (I stared at the examples for quite some time and had to read the description like 4 times, before I got the idea => means your description is incomplete)

    Other than that:

    • please you the new test framework
    • the fixed tests are taking the order of the output in account, while the random tests and the description do/tell the opposite.

    cheers

    • Fbasham Avatar

      Good points. I think I'll unpublish and re-publish to change the behaviour slightly.

    • Blind4Basics Avatar

      :+1:

      note: "please use the new test framework", ofc... x)

    • Fbasham Avatar

      For sure. I wasn't even aware there was v1 or v2. Never authored before.

    • FArekkusu Avatar

      @Fbasham, I've migrated the tests to the new test framework, changed the description a little, and rewrote the example to be more clear about what is grouped and what is aggregated. If it turns out that you were editing the kata yourself right now and overwrite my changes, everything can be restored from the revisions page.

    • Blind4Basics Avatar

      :+1:

      Changes look good to me. Waiting for the feedback of the author before closing, so that we are sure about what is the actual version "online".

    • Fbasham Avatar

      So I've changed the kata slightly to include the group keys in the final output. Sorting of the returned value is also necessary by the user. Sorry @FArekkusu if I've modified your code. Didn't realize anyone was working on this.

    • Blind4Basics Avatar

      the fixed tests are taking the order of the output in account, while the random tests and the description do/tell the opposite.

      this part isn't handled.

      FArekkusu's changes to the description were much better. You should take those (through the revision page... But that might be a pain to extract the data from there...)

      Didn't realize anyone was working on this.

      Thing is... he wasn't supposed to... x)

    • Blind4Basics Avatar

      oh, you actually totally changed the requirements... Let's look at those, then

    • Blind4Basics Avatar

      Well, it's almost worse than it was... ;)

      • you still don't explain how to actually compute the aggregated values
      • if you return the keys with the output, the output type should be [ tuple(key as tuple, list of aggregates), ...]. The current ouput makes the key and the aggregated result indiscernible.

      EDIT: actually, the output should could just be a dict...

    • Fbasham Avatar

      Lol alright fair enough. I think @FArekkusu is right in the the return value should be a dict. I'll find his original description changes and modify that as well.

    • Fbasham Avatar

      Hey dude, I believe everything is now in order.

    • Blind4Basics Avatar

      seems so.

      Issue marked resolved by Blind4Basics 5 years ago
  • FArekkusu Avatar

    The user can modify the input.