mboehm7 commented on pull request #893: URL: https://github.com/apache/systemml/pull/893#issuecomment-623011729
LGTM - thanks @kev-inn this is a great generalization. During the merge I slightly changed the `FrameObject`/`MatrixObject` integration by moving the `fedMapping` meta data and related methods up to `CacheableData` as it's the same for matrices and frames. Similarly, I brought the read from federated frames into the same form as federated matrices. Finally, there were some minor issues such as code duplication (parsing of strings to value-type-specific objects), unnecessary imports, and missing static modifiers of methods which don't access class members. But again, all minor issues - overall this is great. Now we can move to federated data preparation (transform). ---------------------------------------------------------------- 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]
