[GitHub] [madlib] kaknikhil commented on a change in pull request #524: DL: TensorBoard Support

[GitBox <gi...@apache.org>]

kaknikhil commented on a change in pull request #524:
URL: https://github.com/apache/madlib/pull/524#discussion_r589797079



##########
File path: src/ports/postgres/modules/deep_learning/madlib_keras_wrapper.py_in
##########
@@ -145,6 +145,7 @@ def set_model_weights(segment_model, serialized_weights):
 
 """
 Used to convert compile_params and fit_params to actual argument dictionaries
+If strip_quotes is True, each value in the dictionayr will be stripped of quotes

Review comment:
       typo in dictionayr

##########
File path: src/ports/postgres/modules/deep_learning/test/unit_tests/test_madlib_keras.py_in
##########
@@ -1259,6 +1259,26 @@ class MadlibKerasWrapperTestCase(unittest.TestCase):
             self.subject.parse_and_validate_compile_params(test_str)
         self.assertIn('invalid optimizer', str(error.exception))
 
+    def test_parse_callbacks_pass(self):
+        test_str = """'[TensorBoard(log_dir="/tmp/logs/fit")]'"""
+        import utilities
+        self.util = utilities
+        self.util.is_superuser = MagicMock(return_value=True)

Review comment:
       we can also add a test for when `is_superuser` returns false. But we can also do that in a future PR.

##########
File path: src/ports/postgres/modules/deep_learning/test/unit_tests/test_madlib_keras.py_in
##########
@@ -1259,6 +1259,26 @@ class MadlibKerasWrapperTestCase(unittest.TestCase):
             self.subject.parse_and_validate_compile_params(test_str)
         self.assertIn('invalid optimizer', str(error.exception))
 
+    def test_parse_callbacks_pass(self):
+        test_str = """'[TensorBoard(log_dir="/tmp/logs/fit")]'"""

Review comment:
       maybe also add a couple of other tensorboard params here other than log_dir. If it's not straightforward, then feel free to skip it




----------------------------------------------------------------
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:
users@infra.apache.org