Skip to content
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

Removes inputRef / bookmark params from appendRecord #3597

Merged

Conversation

rhyolight
Copy link
Member

Fixes #3592

builds upon #3593

@rhyolight
Copy link
Member Author

rhyolight commented May 9, 2017

@scottpurdy I wanted to get a quick review of the removal of inputRef as a parameter to RecordStream.appendRecord and RecordStream.appendRecords (and the functions implementing those abstract functions).

I don't see where this parameter is used anywhere in our code or tests.

The rest of the changes in this PR are just docstring updates.

@@ -303,7 +304,7 @@ def getNextRecordIdx(self):


@abstractmethod
def appendRecord(self, record, inputRef=None):
def appendRecord(self, record):
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed inputRef here in the abstract method.

@@ -313,7 +314,7 @@ def appendRecord(self, record, inputRef=None):


@abstractmethod
def appendRecords(self, records, inputRef=None, progressCB=None):
def appendRecords(self, records, progressCB=None):
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed inputRef here in the abstract method.

for record in records:
self.appendRecord(record, None)
self.appendRecord(record)
Copy link
Member Author

Choose a reason for hiding this comment

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

No need to send None anymore.

@@ -418,22 +418,17 @@ def appendRecord(self, record, inputBookmark=None):
self._recordCount += 1


def appendRecords(self, records, inputRef=None, progressCB=None):
def appendRecords(self, records, progressCB=None):
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed inputRef here in the implementation method.

@@ -387,7 +387,7 @@ def getNextRecord(self, useCache=True):
return record


def appendRecord(self, record, inputBookmark=None):
def appendRecord(self, record):
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed inputRef here in the implementation method.

"""
return self._aggMonthsAndSeconds


def appendRecord(self, record, inputRef=None):
"""Saves the record in the underlying storage."""
def appendRecord(self, record):
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed inputRef here in the implementation method.

raise RuntimeError("Not implemented in StreamReader")


def appendRecords(self, records, inputRef=None, progressCB=None):
"""Saves multiple records in the underlying storage."""
def appendRecords(self, records, progressCB=None):
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed inputRef here in the implementation method.

@rhyolight
Copy link
Member Author

AppVeyor is stalled, and I'm not waiting for it.

@rhyolight rhyolight merged commit e7cd8cb into numenta:master May 9, 2017
@rhyolight rhyolight deleted the remove-recordstream-appendrecord-bookmark branch May 9, 2017 21:31
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.

2 participants