refactor makeclusters to make it work

[stevenhua0320]

No description provided.

[Ainamacar]

(Edit: I originally posted this comment in Issues #64, but it's more appropriate here.)

I believe this commit will cause breaking changes. The main extraction loop of SrMise (located in extract_single() in peakextraction.py) currently relies on DataClusters following Python's iterator protocol. This commit is somewhere in between: DataClusters still defines __iter__ but since next() no longer exists it won't behave as intended when using an Iterator. (Looking at the docs, I also see that Python 3 iterators should implement a__next__ method instead of the next method used in Python 2.)

What is the desired scope of this refactor?

If it's mainly to make makeclusters() more easily understood, then this is on the right track, we just need to have a __next()__ method that behaves like next() did previously.

However, if it's so DataClusters no longer implements the iterator protocol (which I had assumed in my Issues #64 comments), then further changes are necessary. I regret not mentioning some of these originally, and not noticing others until now.

• The __iter__ method of DataClusters should be removed
• All reference to the StopIteration exception should be removed from cluster_next_point() and a different method used to control when iteration stops. (My initial thought would be to have the function return True if more points remain and False if no points remain or the object is in an invalid state.)
• The main extraction loop would need to be altered. (For example, if cluster_next_point() returns True or False, something like while dclusters.cluster_next_point(): should work, although this is a bit more fragile than the iterator approach.)
• Likely the step variable should be removed in favor of using dclusters only.
• The comment in extract_single() about DataClusters being an iterator should be modified.

[sbillinge]

this might be a good place to "try out" the py3 refactor.

@stevenhua0320 step 1 would be to write a unit test for the behavior that is breaking. Then we can think about what code would satisfy that test that uses built-in iterators? Worth a try?

[stevenhua0320]

Simon could you revert this merged commit right now? Since Luke said in the comment that this commit might cause breaking changes, I'm not sure right now if this would affect other files that depend on DataClusters class. For PR #75 it is just change the syntax from py2 to py3, so that is a safe refactor, and we should now use that.