njayaram2 commented on a change in pull request #360: Deep Learning: Add support for one-hot encoded dep var URL: https://github.com/apache/madlib/pull/360#discussion_r270978404
########## File path: src/ports/postgres/modules/deep_learning/madlib_keras.py_in ########## @@ -31,103 +31,21 @@ from keras.models import * from keras.optimizers import * from keras.regularizers import * -from madlib_keras_helper import KerasWeightsSerializer +from madlib_keras_helper import FitInputValidator Review comment: I guess this is best addressed when the future commit happens. But, I am not sure why we need a new file called `madlib_keras_serializer.py_in`. I think it's fine as is, as a class in the helper function. If we have another class to validate predict, we can have a separate class for that. That would also have validate input code, and it might be a good idea to containerize these functions for fit and predict inside their own classes. Moreover, we also have `validate_input_shapes` as one of the functions in `class FitInputValidator`. ---------------------------------------------------------------- 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] With regards, Apache Git Services
