mboehm7 commented on pull request #904: URL: https://github.com/apache/systemml/pull/904#issuecomment-628910995
LGTM - thanks @lukas-jkl for this awesome patch, and @Baunsgaard for the detailed discussions. Overall, this is a great initial importer and completes the AMLS project. During the merge, I only added `**/*.out` to the exclude list of the rat check (for proper licenses). Down the the road, I would also prefer to remove these expected output files and rather generate them dynamically (e.g., via pytorch). Similarly, we should clean up the license headers to use a common text layout (beginning/end lines). Btw, there are additional rat problems in the data slicing implementation (which I fix when merging Svetlana's PR) and the `JolEstimate*` tests (@Baunsgaard you might want to fix that). ---------------------------------------------------------------- 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]
