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

Make region file names snake_case (part 7) #3640

Merged
merged 15 commits into from
May 24, 2017

Conversation

rhyolight
Copy link
Member

@rhyolight rhyolight commented May 22, 2017

Fixes #3641

Do not merge. Tests are failing. Posting here to show the example.

Vitaly Kruglikov added 2 commits May 16, 2017 13:17
…te the

builtin regions registration to demonstrate failure witnessed by Matt
…nit__.py to match the renamed module

to demonstrate that the class name of the python region may differ from its module name, as long as it's
registered using the correct module name.
@@ -49,7 +49,7 @@
("nupic.regions.KNNClassifierRegion", "KNNClassifierRegion"),
("nupic.regions.PluggableEncoderSensor", "PluggableEncoderSensor"),
("nupic.regions.PyRegion", "PyRegion"),
("nupic.regions.RecordSensor", "RecordSensor"),
Copy link
Member Author

Choose a reason for hiding this comment

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

@vitaly-krugl @scottpurdy Here I have updated the region registration, but still getting a test error:

> python tests/unit/nupic/regions/record_sensor_region_test.py
ERR:  CHECK FAILED: "pClass && PyType_Check(pClass)"  [/Users/mtaylor/nta/nupic.core/src/nupic/py_support/PyHelpers.cpp line 656]
WARN:   PyRegion::createSpec failed: 0x7fff521cddd8
EERR:  CHECK FAILED: "pClass && PyType_Check(pClass)"  [/Users/mtaylor/nta/nupic.core/src/nupic/py_support/PyHelpers.cpp line 656]
WARN:   PyRegion::createSpec failed: 0x7fff521cddd8
EERR:  CHECK FAILED: "pClass && PyType_Check(pClass)"  [/Users/mtaylor/nta/nupic.core/src/nupic/py_support/PyHelpers.cpp line 656]
WARN:   PyRegion::createSpec failed: 0x7fff521cdf48
E
======================================================================
ERROR: testActValueOut (__main__.RecordSensorRegionTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/unit/nupic/regions/record_sensor_region_test.py", line 118, in testActValueOut
    network = _createNetwork()
  File "tests/unit/nupic/regions/record_sensor_region_test.py", line 36, in _createNetwork
    network.addRegion('sensor', 'py.record_sensor', '{}')
  File "/Users/mtaylor/nta/nupic/src/nupic/engine/__init__.py", line 641, in addRegion
    engine_internal.Network.addRegion(self, name, nodeType, nodeParams)
  File "/Users/mtaylor/nta/nupic.core/bindings/py/src/nupic/bindings/engine_internal.py", line 1167, in addRegion
    return _engine_internal.Network_addRegion(self, *args, **kwargs)
AttributeError: 'module' object has no attribute 'record_sensor'

======================================================================
ERROR: testBucketIdxOut (__main__.RecordSensorRegionTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/unit/nupic/regions/record_sensor_region_test.py", line 111, in testBucketIdxOut
    network = _createNetwork()
  File "tests/unit/nupic/regions/record_sensor_region_test.py", line 36, in _createNetwork
    network.addRegion('sensor', 'py.record_sensor', '{}')
  File "/Users/mtaylor/nta/nupic/src/nupic/engine/__init__.py", line 641, in addRegion
    engine_internal.Network.addRegion(self, name, nodeType, nodeParams)
  File "/Users/mtaylor/nta/nupic.core/bindings/py/src/nupic/bindings/engine_internal.py", line 1167, in addRegion
    return _engine_internal.Network_addRegion(self, *args, **kwargs)
AttributeError: 'module' object has no attribute 'record_sensor'

======================================================================
ERROR: testVaryingNumberOfCategories (__main__.RecordSensorRegionTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/unit/nupic/regions/record_sensor_region_test.py", line 72, in testVaryingNumberOfCategories
    "sensor", "py.record_sensor", "{'numCategories': 2}")
  File "/Users/mtaylor/nta/nupic/src/nupic/engine/__init__.py", line 641, in addRegion
    engine_internal.Network.addRegion(self, name, nodeType, nodeParams)
  File "/Users/mtaylor/nta/nupic.core/bindings/py/src/nupic/bindings/engine_internal.py", line 1167, in addRegion
    return _engine_internal.Network_addRegion(self, *args, **kwargs)
AttributeError: 'module' object has no attribute 'record_sensor'

----------------------------------------------------------------------
Ran 3 tests in 0.005s

FAILED (errors=3)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be:

("nupic.regions.record_sensor", "RecordSensor"),

Copy link
Member Author

Choose a reason for hiding this comment

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

Tried that first, same problem.

Copy link
Member

Choose a reason for hiding this comment

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

@rhyolight: @scottpurdy is right. See my PR that demonstrated the same change that you're attempting to make and built successfully in all environments: https://github.com/numenta/nupic/pull/3632/files.

Please compare your changes against my PR or reopen my PR and use it, instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

See other comments on a few of the places that need to be updated to "py.RecordSensor". Can you change this back to my suggestion so I can see everything in context. I'm not sure which permutations you tried.

@@ -1,7 +1,7 @@
import json

# Add a sensor region, set its encoder and data source.
network.addRegion("sensor", "py.RecordSensor", json.dumps({"verbosity": 0}))
network.addRegion("sensor", "py.record_sensor", json.dumps({"verbosity": 0}))
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be "py.RecordSensor"

@@ -68,7 +68,7 @@ def createNetwork(dataSource):
# Our input is sensor data from the gym file. The RecordSensor region
# allows us to specify a file record stream as the input source via the
# dataSource attribute.
network.addRegion("sensor", "py.RecordSensor",
network.addRegion("sensor", "py.record_sensor",
Copy link
Contributor

Choose a reason for hiding this comment

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

"py.RecordSensor"

@@ -135,7 +135,7 @@ def createRecordSensor(network, name, dataSource):
"""

# Specific type of region. Possible options can be found in /nupic/regions/
regionType = "py.RecordSensor"
regionType = "py.record_sensor"
Copy link
Contributor

Choose a reason for hiding this comment

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

"py.RecordSensor"

@rhyolight
Copy link
Member Author

@scottpurdy Figured it out, thanks. I changed the wrong strings.

Copy link
Member

@vitaly-krugl vitaly-krugl left a comment

Choose a reason for hiding this comment

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

LGTM, assuming tests pass.

@@ -49,7 +49,7 @@
("nupic.regions.KNNClassifierRegion", "KNNClassifierRegion"),
("nupic.regions.PluggableEncoderSensor", "PluggableEncoderSensor"),
("nupic.regions.PyRegion", "PyRegion"),
("nupic.regions.RecordSensor", "RecordSensor"),
Copy link
Member

Choose a reason for hiding this comment

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

@rhyolight: @scottpurdy is right. See my PR that demonstrated the same change that you're attempting to make and built successfully in all environments: https://github.com/numenta/nupic/pull/3632/files.

Please compare your changes against my PR or reopen my PR and use it, instead.

@@ -58,7 +58,7 @@ def createNetwork(dataSource):
network = Network()

# Add a sensor region.
network.addRegion("sensor", "py.RecordSensor", '{}')
network.addRegion("sensor", "py.record_sensor", '{}')
Copy link
Member

Choose a reason for hiding this comment

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

@rhyolight, this change and othes like it are the culprit. You should leave them alone. "py.RecordSensor" refers to the class name, not the module name. The registration logic then locates the module name that matches the class name.

@rhyolight
Copy link
Member Author

@scottpurdy @vitaly-krugl Ok guys. I'm going to add the rest of the region name changes to this PR. More commits coming.

@rhyolight rhyolight changed the title WIP Move RecordSensor.py --> record_sensor.py Make region file names snake_case (part 7) May 23, 2017
rhyolight added 10 commits May 23, 2017 11:12
    renamed:    src/nupic/regions/KNNClassifierRegion.py ->
src/nupic/regions/knn_classifier_region.py
    renamed:    src/nupic/regions/PluggableEncoderSensor.py ->
src/nupic/regions/pluggable_encoder_sensor.py
    renamed:    src/nupic/regions/SDRClassifierRegion.py ->
src/nupic/regions/sdr_classifier_region.py
    renamed:    src/nupic/regions/SPRegion.py ->
src/nupic/regions/sp_region.py
    renamed:    src/nupic/regions/TMRegion.py ->
src/nupic/regions/tm_region.py
RecordSensorFilter -> record_sensor_filters
SVMClassifierNode -> svm_classifier_node
TestRegion -> test_region
UnimportableNode -> unimportable_node
@rhyolight
Copy link
Member Author

@scottpurdy or @vitaly-krugl This is ready for final review.

Copy link
Contributor

@scottpurdy scottpurdy left a comment

Choose a reason for hiding this comment

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

Great work!

@rhyolight rhyolight merged commit acd3e00 into numenta:master May 24, 2017
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