Refactor server code to facilitate error checking on multiple formats

[dondi]

While implementing our first import function (for SIF) I noticed that many error checks that are performed for the original .xlsx format will apply to other imports as well. However, the current code structure prevents those from being leveraged easily. The code will have to be refactored this way:

• All file-reading routines will first target an intermediate JSON representation consisting solely of nodes, edges, and weights
• This uniform representation can then be sent through a pipeline of checkers (e.g., name lengths, number of nodes, etc.) that can be uniform and independent of the original file format
• Other activities can also happen after the file is read; specifically, the post-processing of the edges that determines the arrowhead and color (this might actually be more practical to do on the client side)
• Errors can certainly still happen while actually reading the file, and for those errors, the code doesn't have to change. But by moving the other error/warning checks until after the file is read, we thus restrict the errors that happen during file processing to just errors in the expected format (syntactic errors, in a way) vs. errors that are independent of format (analogous but not equivalent to semantic errors)

[kdahlquist]

A reminder from issue #316:

"Node names must be 12 characters or fewer." —The import code does not enforce this yet; it’s one of the checks that needs to be refactored out of the current GRN-loading code.

[kdahlquist]

Checker code needs to be moved out of Excel function.

• Don't change the unit tests first.
• Perform the refactor to see that tests don't break.
  • Figure out what tests can be performed when it is already JSON.
  • Ideally it would be all, but some things might be tied to the way the files are being read.
• Then can expand the test suite to include loading GraphML and SIF.

[kdahlquist]

@yshin4 and @mihirsamdarshi will now start working on this issue.

• [x] identify which errors are semantic and which are syntactic
• [x] do the refactor to catch the semantic errors
• [ ] make the test files for SIF and GraphML that are not semantic (this is now issue #362 and #363.

[kdahlquist]

@yshin4 will put her list of semantic vs. syntactic errors on the wiki.

Then she and @mihirsamdarshi can divvy up the semantic ones and start refactoring them.

[yshin4]

posted to wiki. Will meet with @mihirsamdarshi and discuss.

[kdahlquist]

When you guys work on the dataTypeError, you should keep in mind issue #343 where @NAnguiano reported that the graph statistics function of Cytoscape won't allow special characters except for "-" and "_". Even though GRNsight can now handle them, when we implement the graph statistics, it will become a problem. It is probably better to just go ahead and restrict it now instead of waiting. This means that whenever GRNsight discovers a special character, it will need to error out and report to the user that it is not allowed.

[yshin4]

Added specialCharacterError, will implement it in Try&catch in spreadsheet controller.

[kdahlquist]

1. Create semanticChecker function
2. remove code from Excel and put it into semanticChecker function
3. call semanticChecker function from SIF and GraphML

@mihirsamdarshi should start with the Gene name length check because it is an easy one to do.

Also read the documentation page to get familiar with SIF and GraphML formats.

[yshin4]

Done coding in specialCharacterError function and implementing it in Try and Catch. But I don't want to break anything so I'll check with other team members on Monday before pushing.

[yshin4]

Implemented special character error. Edited both spreadsheet controller and test files. Updated Error Sort on Wiki.

[kdahlquist]

semanticChecker is almost working; Most tests are passing except for 3, they need to talk to @dondi in office hours this week.

They need source genes and target genes, but they can't get them both from network.

[kdahlquist]

The issue from the previous comment is still there; they need to talk to @dondi .

[dondi]

@mihirsamdarshi and @yshin4 can look at the (failing) error list then track down where the spurious errors could have been added. Following those should eventually lead to the offending code.

[dondi]

This is a good example for why test cases should be as narrow as possible, because another potential reason for the spurious error(s) is that the test data itself unintentionally has more semantic errors than intended. Ideally the test data should be formulated so that they are perfect except for the single error type being tested.

[dondi]

@yshin4 and @dondi met on Tuesday evening and determined that the core issue with @yshin4's duplicate check errors is that only the Excel format has a notion of row vs. column. Thus there are actually two types of duplicate errors: one that can differentiate whether the duplication occurred along a row or column dimension, and one that determines a duplication within the overall list of gene names. The former is thus more of a syntax error because it is file format dependent, and the latter remains as a semantic error.

Given this observation, @yshin4 will implement duplication checks at both levels: for the Excel reader only, we can detect duplications in rows vs. columns distinctly. For all other importers which do not have this concept, we will wait until the consolidated list of genes emerges from the import and then do a duplication check from that list.

[kdahlquist]

All of the cases on the wiki here have been implemented: https://github.com/dondi/GRNsight/wiki/Error-Sort

So, question for @dondi : are there additional semantic tests that need to be written or can we safely close this now because we have new issues for syntactic tests of graphml and sif #362 and #363.

[dondi]

I inspected the code and this specific issue can be closed. However I noticed that the semantic checker has only been integrated into the Excel reader, and not SIF nor GraphML. I will create those as separate issues.