Skip to content

Conversation

@looselycoupled
Copy link
Member

No description provided.


# TODO: allow this at some future point
if agg == "all":
raise AttributeError("cannot use 'all' as aggregate at this time")
Copy link
Contributor

Choose a reason for hiding this comment

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

What would happen if agg is something other than mean, min, max, etc?

Copy link
Member Author

Choose a reason for hiding this comment

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

KeyError I believe. I'm fine with not protecting against that.

item[col] = getattr(row[idx], agg) if row[idx] else None
if agg == "all":
for stat in _STAT_PROPERTIES:
item["{}-{}".format(col, stat)] = getattr(row[idx], stat) if row[idx] else None
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

raise AttributeError("cannot use 'all' as aggregate at this time")

if not callable(name_callable):
name_callable = lambda s: s.collection + "/" + s.name
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to raise something here? It might be confusing if the user expects the columns to have custom names, but they aren't appearing and they wouldn't know why?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm.. this is mostly checking for None and making sure they user gave us something valid. I'll add a ticket to rethink this in the future.

"""
assert to_dateframe uses name lambda
"""
columns = ["stream0", "stream1", "stream2", "stream3"]
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really understand the purpose of columns here?

Copy link
Member Author

Choose a reason for hiding this comment

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

yup, thats leftover. I've deleted in a new commit

@looselycoupled looselycoupled merged commit dbdd7eb into develop May 13, 2020
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.

3 participants