Skip to content

[SYSTEMML-621] Remove redundant nested error messages#255

Closed
deroneriksson wants to merge 2 commits intoapache:masterfrom
deroneriksson:SYSTEMML-621-remove_nested_error_messages
Closed

[SYSTEMML-621] Remove redundant nested error messages#255
deroneriksson wants to merge 2 commits intoapache:masterfrom
deroneriksson:SYSTEMML-621-remove_nested_error_messages

Conversation

@deroneriksson
Copy link
Member

Addresses some redundant nested error messages for read, rand, and matrix statements.

Given the following DML:

a=read();
b=matrix();
c=rand(2,2);

This previously generated the following output:

ERROR a.dml line 1:0 unable to process builtin function expression read:ERROR: a.dml -- line 1, column 0 -- read method must have at least filename parameter
ERROR a.dml line 2:0 unable to process builtin function expression matrix:ERROR: a.dml -- line 2, column 0 -- for matrix statement, must specify at least 3 arguments (in order): data, rows, cols
ERROR a.dml line 3:0 unable to process builtin function expression rand:ERROR: a.dml -- line 3, column 0 -- for Rand Statement all arguments must be named parameters

--------------------------------------------------------------
The following 3 parse issues were encountered:
#1 a.dml [line 1:0] [Validation error] -> a=read();
   unable to process builtin function expression read:ERROR: a.dml -- line 1, column 0 -- read method must have at least filename parameter
#2 a.dml [line 2:0] [Validation error] -> b=matrix();
   unable to process builtin function expression matrix:ERROR: a.dml -- line 2, column 0 -- for matrix statement, must specify at least 3 arguments (in order): data, rows, cols
#3 a.dml [line 3:0] [Validation error] -> c=rand(2,2);
   unable to process builtin function expression rand:ERROR: a.dml -- line 3, column 0 -- for Rand Statement all arguments must be named parameters
--------------------------------------------------------------

With this PR it generates:

ERROR a.dml line 1:0 read method must have at least filename parameter
ERROR a.dml line 2:0 for matrix statement, must specify at least 3 arguments: data, rows, cols
ERROR a.dml line 3:0 for rand statement, all arguments must be named parameters

--------------------------------------------------------------
The following 3 parse issues were encountered:
#1 a.dml [line 1:0] [Validation error] -> a=read();
   read method must have at least filename parameter
#2 a.dml [line 2:0] [Validation error] -> b=matrix();
   for matrix statement, must specify at least 3 arguments: data, rows, cols
#3 a.dml [line 3:0] [Validation error] -> c=rand(2,2);
   for rand statement, all arguments must be named parameters
--------------------------------------------------------------

// handle first parameter, which is data and may be unnamed
ParameterExpression firstParam = passedParamExprs.get(0);
if (firstParam.getName() != null && !firstParam.getName().equals(DataExpression.RAND_DATA)){
// throw exception -- must be filename as first parameter
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Deron - should this 'throw exception' comment also be removed similar to change made in line 278? Thanks, Glenn

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good eye! Yes the comment should be removed too. I'll remove it.

@deroneriksson
Copy link
Member Author

Obsolete comment removed.

@gweidner
Copy link
Member

Thank you @deroneriksson for simplifying the error messages!

LGTM

@deroneriksson
Copy link
Member Author

@gweidner Thanks for reviewing! I'll merge.

@asfgit asfgit closed this in 8dda69c Sep 23, 2016
j143-zz pushed a commit to j143-zz/systemml that referenced this pull request Nov 4, 2017
Remove some nested error messages for read, matrix, and rand statements.

Closes apache#255.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants