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

Added Serializable to all classes with a capnp write function #3710

Merged
merged 5 commits into from
Jun 15, 2017

Conversation

rhyolight
Copy link
Member

There is a problem with this PR, tests will fail with this error, but I'm not sure why:

============================================== ERRORS ==============================================
_________________________ ERROR collecting tests/unit/nupic/utils_test.py __________________________
tests/unit/nupic/utils_test.py:28: in <module>
    from nupic.utils import MovingAverage
src/nupic/utils.py:32: in <module>
    class MovingAverage(object, Serializable):
/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/abc.py:87: in __new__
    cls = super(ABCMeta, mcls).__new__(mcls, name, bases, namespace)
E   TypeError: Error when calling the metaclass bases
E       Cannot create a consistent method resolution
E   order (MRO) for bases object, Serializable
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: stopping after 1 failures !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
===================================== 1 error in 0.22 seconds ======================================

@lscheinkman
Copy link
Contributor

The MovingAverage class does not implement the abstract method getSchema

@rhyolight
Copy link
Member Author

@lscheinkman Even after that change, I still get the same error on MovingAverage.

@lscheinkman
Copy link
Contributor

remove object from the mixin list. Serializable already inherits from object.

class MovingAverage(Serializable):
  """

@rhyolight
Copy link
Member Author

Thanks @lscheinkman that was it.

@rhyolight rhyolight requested a review from lscheinkman June 15, 2017 19:19

@classmethod
def getSchema(cls):
raise NotImplementedError()
Copy link
Contributor

Choose a reason for hiding this comment

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

Should return MovingAverageProto from this import

from nupic.movingaverage_capnp import MovingAverageProto

Copy link
Contributor

@lscheinkman lscheinkman left a comment

Choose a reason for hiding this comment

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

@rhyolight
Copy link
Member Author

@lscheinkman ready

Copy link
Contributor

@lscheinkman lscheinkman left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@rhyolight
Copy link
Member Author

investigating test failure 🤷‍♂️

@lscheinkman
Copy link
Contributor

You need to add capnp conditionals for Windows. Something like this:

try:
   import capnp
 except ImportError:
   capnp = None
if capnp:
  from nupic.movingaverage_capnp import MovingAverageProto

@rhyolight rhyolight merged commit 9641434 into numenta:master Jun 15, 2017
@rhyolight rhyolight deleted the serializable branch June 15, 2017 21:41
@rhyolight
Copy link
Member Author

@lscheinkman thanks for the help

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