temporal_memory.py excitedColumnsGenerator() Has No Unit Test

[cogmission]

Please correct me if I'm wrong, but I'm thinking excitedColumnsGenerator() method is central to the operation of the new TemporalMemory algorithm implementation - and I didn't find a unit test for it?

[mrcslws]

It's usually best for unit tests to test all units via the public interface. The excitedColumnsGenerator is the equivalent of a private method. So the unit tests should cover this function via calls to the compute method, and I think they do.

There are exceptions to this rule, of course.

[cogmission]

It's usually best for unit tests to test all units via the public interface.
There are exceptions to this rule, of course.

Rather than argue the universality of this truth, I respectfully submit that this being a Python Generator method that is very complex almost mandates that a test is created to exercise the bounds of its functionality and demonstrate its behavior to users - since this is "core" to the idiom used to implement the algorithm? I know I'm looking at it for the first time and would appreciate all the help I can get to understand your "wizardry" here. ;-)

Also I have questions as to whether this is a classic generator in that it persists state which is re-assumable at subsequent calls? Instead of me asking you this, though - I would like to be able to look at the code (...test, and documentation) and clearly see the answer to this?

If you really feel that if I were a seasoned Python person, it would be immediately understandable then I will yield the point... But I suspect otherwise?

[andrewmalta13]

Yeah it is a non-trivial function to understand. I wouldn't be opposed to write a test for it and document it better. I am working on a pr for this code right now I can include it in that. It is in fact a classic generator and, as such, maintains the state of the function between calls.

[rhyolight]

Thanks @andrewmalta13. Be sure to add fixes #3230 to the PR description somewhere (in addition to whatever other issue it fixes).