mboehm7 commented on pull request #1369: URL: https://github.com/apache/systemds/pull/1369#issuecomment-922481472
LGTM - thanks @fathollahzadeh for finalizing this initial version of the IOGEN framework. So far this implementation uses generalized readers for types of structured inputs - it's fine to postpone the actual code generation until we come to structure that requires generation for performance. During the merge I made a number of minor changes: * Included the iogen tests into our github test workflow * Fixed two failing tests (set to ignore) which missed some input file. Similarly other tests throw exceptions due to missing files but succeed. Please do a separate PR for fixing these tests including better checks for expected results. * Fixed a number of warnings of unused variables, and simplified some verbose code snippets * Fixed the parsing of ints via the fast string tokenizer (instead of `(int)Double.parseDouble()` we should use `Integer.parseInt()`) * In the future please avoid wildcard imports and reformatting files (e.g., `DataConverter` here) where no changes have been made. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
