Skip to content

Refactor StreamSets and the chaining [ch13505] #8

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Refactor StreamSets and the chaining [ch13505] #8

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b1059ca
TBD Discussion on StreamSets and the chaining [ch13505]
jleifnf Jul 1, 2021
86bafc0
fix TypeError for tuple unpacking
jleifnf Jul 2, 2021
3bfba3d
fix TypeError for tuple unpacking (2)
jleifnf Jul 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 54 additions & 2 deletions btrdb/stream.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1107,13 +1107,19 @@ def clone(self):
setattr(clone, attr, deepcopy(val))
return clone

def windows(self, width, depth):
def windows(self, *args, **kwargs):
Copy link

Choose a reason for hiding this comment

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

I think the original version (using specific args, with expressive variable names) was clearer to understand and use, and simplified the code since it didn't need the if/else statements below. Is there a way we can implement the functionality you want without using *args & *kwargs?

"""
Stores the request for a windowing operation when the query is
eventually materialized.

Parameters
----------
start : int or datetime like object
the inclusive start of the query (see :func:`btrdb.utils.timez.to_nanoseconds`
for valid input types)
end : int or datetime like object
the exclusive end of the query (see :func:`btrdb.utils.timez.to_nanoseconds`
for valid input types)
width : int
The number of nanoseconds to use for each window size.
depth : int
Expand Down Expand Up @@ -1145,6 +1151,26 @@ def windows(self, width, depth):
specified. This is much faster to execute on the database side.

"""
start, end = None, None
if len(args) < 2 and len(kwargs) < 2:
raise UserWarning("Require 'width' and 'depth'")
elif len(args) == 2 and len(kwargs) == 0:
Copy link

Choose a reason for hiding this comment

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

Seems surprising to me that the user can specify width and depth in either the args or the kwargs. Also, it might be unclear to the user how to order width & depth when specifying as args.

width, depth = args
elif len(args) == 0 and len(kwargs) == 2:
Copy link

@chrisjryan chrisjryan Jul 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I think that, if the user gave start & end in the kwargs but not width & depth, this could lead to unclear errors.

width, depth = kwargs.get('width', None), kwargs.get('depth', None)
elif len(args) == 0 and len(kwargs) == 4:
start, end, width, depth = [kwargs.get(k, None) for k in ('start', 'end', 'width', 'depth')]
elif len(args) == 4 and len(kwargs) == 0:
Copy link

@chrisjryan chrisjryan Jul 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I think overall that having multiple ways to order & submit the input args could add more confusion than usability.

start, end, width, depth = args
else:
if kwargs.get('width', None) and kwargs.get('depth', None):
start, end = args
width, depth = kwargs.get('width', None), kwargs.get('depth', None)
else:
width, depth = args
start, end = kwargs.get('start', None), kwargs.get('end', None)
if start is not None and end is not None:
self = self.filter(start, end) # because the `filter` returns an copy object.
if not self.allow_window:
raise InvalidOperation("A window operation is already requested")

Expand All @@ -1153,13 +1179,19 @@ def windows(self, width, depth):
self.depth = int(depth)
return self

def aligned_windows(self, pointwidth):
def aligned_windows(self, *args, **kwargs):
"""
Stores the request for an aligned windowing operation when the query is
eventually materialized.

Parameters
----------
start : int or datetime like object
the inclusive start of the query (see :func:`btrdb.utils.timez.to_nanoseconds`
for valid input types)
end : int or datetime like object
the exclusive end of the query (see :func:`btrdb.utils.timez.to_nanoseconds`
for valid input types)
pointwidth : int
The length of each returned window as computed by 2^pointwidth.

Expand All @@ -1181,6 +1213,26 @@ def aligned_windows(self, pointwidth):
omitted.

"""
start, end = None, None
if len(args) < 1 and len(kwargs) < 1:
raise UserWarning("Require 'pointwidth'")
elif len(args) == 1 and len(kwargs) == 0:
pointwidth = args[0]
elif len(args) == 0 and len(kwargs) == 1:
pointwidth = kwargs.get('pointwidth', None)
elif len(args) == 0 and len(kwargs) == 3:
start, end, pointwidth = [kwargs.get(k, None) for k in ('start', 'end', 'pointwidth')]
elif len(args) == 3 and len(kwargs) == 0:
start, end, pointwidth = args
else:
if kwargs.get('pointwidth', None):
start, end = args
pointwidth = kwargs.get('pointwidth', None)
else:
pointwidth = args[0]
start, end = kwargs.get('start', None), kwargs.get('end', None)
if start is not None and end is not None:
self = self.filter(start, end) # because the `filter` returns an copy object.
if not self.allow_window:
raise InvalidOperation("A window operation is already requested")

Expand Down