iindyk commented on pull request #13175: URL: https://github.com/apache/beam/pull/13175#issuecomment-760293944
The algorithm remains as is described in the paper. These changes have more to do with the implementation rather than the algorithm itself. The core change here is the Cythonization which has to come in one piece (other optimizations are few line changes and are interconnected). Cythonization requires moving many things outside of ApproximateQuantilesCombineFn (not an extension class and doesn't allow cythonization) which make a bulk of this change. So my feeling is that splitting them will not make it easier to review. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
