Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing issue #1072. #1085

Merged
merged 5 commits into from
Feb 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions server/controllers/importers/sif.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ module.exports = function (sif) {
sifEntries.forEach(function (entry) {
if (entry.length > TARGET) {
if (!isNumber(entry[RELATIONSHIP])) {
if (entry[RELATIONSHIP] !== "pd") {
if (entry[RELATIONSHIP] !== "pd" && entry[RELATIONSHIP] !== "pp") {
unweightedRelationshipTypeErrorDetected = true;
}
}
Expand All @@ -54,6 +54,24 @@ module.exports = function (sif) {
}
});

let hasNumbers = relationships.some(isNumber);
let allNumbers = relationships.every(isNumber);

let networkMode;
if (allNumbers) {
networkMode = "grn";
} else {
for (const relationship of relationships) {
if (relationship === "pp") {
networkMode = "protein-protein-physical-interaction";
break;
} else if (relationship === "pd") {
networkMode = "grn";
break;
}
}
}

if (unweightedRelationshipTypeErrorDetected) {
errors.push(sifConstants.errors.SIF_UNWEIGHTED_RELATIONSHIP_TYPE_ERROR);
}
Expand All @@ -62,9 +80,8 @@ module.exports = function (sif) {
errors.push(sifConstants.errors.SIF_MISSING_DATA_ERROR);
}

var hasNumbers = relationships.some(isNumber);
var allNumbers = relationships.every(isNumber);
return {
networkMode: networkMode,
sheetType: allNumbers ? constants.WEIGHTED : constants.UNWEIGHTED,
warnings: hasNumbers && !allNumbers ? constants.warnings.EDGES_WITHOUT_WEIGHTS : null,
errors: errors
Expand Down Expand Up @@ -134,6 +151,7 @@ module.exports = function (sif) {
errors: errors,
warnings: warnings,
sheetType: workbookType.sheetType,
networkMode: workbookType.networkMode,
positiveWeights: [],
negativeWeights: [],
meta: {},
Expand Down
10 changes: 9 additions & 1 deletion server/controllers/network-sheet-parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,15 @@ var parseNetworkSheet = function (sheet, network) {
var rowData = [];

// check for “cols regulators/rows targets” in cell A1
const cellA1 = sheet.data[0][0];
let cellA1 = "";
try {
cellA1 = sheet.data[0][0];
} catch (err) {
const row = 0;
const column = 0;
addError(network, constants.errors.missingValueError(row, column));
return network;
}

// TODO There are now 2 valid values for cellA1. One indicates GRN, the other is PPI.
// If neither, then we continue with the warning.
Expand Down
8 changes: 4 additions & 4 deletions server/controllers/sif-constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ module.exports = {
SIF_UNWEIGHTED_RELATIONSHIP_TYPE_ERROR: {
errorCode: "SIF_UNWEIGHTED_RELATIONSHIP_TYPE_ERROR",
possibleCause: "The SIF importer detects an unweighted network with an unsupported relationship type.",
suggestedFix: "SIF files accepted by GRNsight must use 'pd' as the text string for the relationship" +
" type in unweighted networks. Please review the SIF input documentation. Additionally, this" +
" error may be have been caused by missing data in your file, which caused the importer to" +
" incorrectly interpret a source or target as the relationship."
suggestedFix: "SIF files accepted by GRNsight must use 'pd' or 'pp' as the text string for the" +
" relationship type in unweighted networks. Please review the SIF input documentation." +
" Additionally, this error may be have been caused by missing data in your file, which caused " +
"the importer to incorrectly interpret a source or target as the relationship."
Comment on lines +16 to +19
Copy link
Owner

Choose a reason for hiding this comment

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

Doesn’t have to change here to retain consistency, but for building new strings I have found that using the join array method feels structurally cleaner

},

SIF_MISSING_DATA_ERROR: {
Expand Down
67 changes: 54 additions & 13 deletions test/import-sif-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ var importController = require(__dirname + "/../server/controllers" + "/import-c
var constants = require(__dirname + "/../server/controllers" + "/constants");
var initWorkbook = require(__dirname + "/../server/controllers" + "/helpers.js").initWorkbook;

var expectedUnweightedWorkbook = initWorkbook({
let expectedUnweightedGRNWorkbook = initWorkbook({
genes: [
{ name: "A" },
{ name: "B" },
Expand All @@ -27,9 +27,35 @@ var expectedUnweightedWorkbook = initWorkbook({
negativeWeights: [],
sheetType: "unweighted",
meta: {},
expression:{}
expression:{},
networkMode: "grn"
});

let expectedUnweightedPPIWorkbook = initWorkbook({
genes: [
{ name: "A" },
{ name: "B" },
{ name: "C" },
{ name: "D" }
],

links: [
{ source: 1, target: 0 },
{ source: 1, target: 2 },
{ source: 2, target: 1 }
],

errors: [],
warnings: [],
positiveWeights: [],
negativeWeights: [],
sheetType: "unweighted",
meta: {},
expression:{},
networkMode: "protein-protein-physical-interaction"
});


var expectedWeightedWorkbook = initWorkbook({
genes: [
{ name: "A" },
Expand All @@ -50,6 +76,7 @@ var expectedWeightedWorkbook = initWorkbook({
negativeWeights: [],
sheetType: "weighted",
meta: {},
networkMode: "grn",
expression:{}
});

Expand All @@ -76,7 +103,8 @@ var expectedUnweightedWorkbookWithCycle = initWorkbook({
negativeWeights: [],
sheetType: "unweighted",
meta: {},
expression:{}
expression:{},
networkMode: "grn"
});

var expectedWeightedWorkbookWithCycle = initWorkbook({
Expand All @@ -102,18 +130,26 @@ var expectedWeightedWorkbookWithCycle = initWorkbook({
negativeWeights: [],
sheetType: "weighted",
meta: {},
expression:{}
expression:{},
networkMode: "grn"
});

// Unweighted SIF

var unweightedTestSif = [
var unweightedGRNTestSif = [
"A",
[ "B", "pd", "A", "C" ].join("\t"),
[ "C", "pd", "B" ].join("\t"),
"D"
].join("\r\n"); // Mix up linebreak types to test normalization.

var unweightedPPITestSif = [
"A",
[ "B", "pp", "A", "C" ].join("\t"),
[ "C", "pp", "B" ].join("\t"),
"D"
].join("\r\n"); // Mix up linebreak types to test normalization.

var unweightedTestSifWithCycle = [
[ "A", "pd", "A" ].join("\t"),
[ "B", "pd", "A", "C" ].join("\t"),
Expand Down Expand Up @@ -283,8 +319,11 @@ var sifWithSemanticErrorOnly = [
describe("Import from SIF", function () {
it("should import unweighted workbooks from SIF correctly", function () {
expect(
importController.sifToGrnsight(unweightedTestSif)
).to.deep.equal(expectedUnweightedWorkbook);
importController.sifToGrnsight(unweightedGRNTestSif)
).to.deep.equal(expectedUnweightedGRNWorkbook);
expect(
importController.sifToGrnsight(unweightedPPITestSif)
).to.deep.equal(expectedUnweightedPPIWorkbook);
});

it("should import weighted workbooks from SIF correctly", function () {
Expand All @@ -296,7 +335,7 @@ describe("Import from SIF", function () {
it("should import inconsistently weighted workbooks from SIF as unweighted", function () {
expect(
importController.sifToGrnsight(inconsistentlyWeightedTestSif)
).to.deep.equal(extend(true, {}, expectedUnweightedWorkbook, {
).to.deep.equal(extend(true, {}, expectedUnweightedGRNWorkbook, {
warnings: [
constants.warnings.EDGES_WITHOUT_WEIGHTS
]
Expand Down Expand Up @@ -350,7 +389,8 @@ describe("Import from SIF", function () {
negativeWeights: [],
sheetType: "unweighted",
meta: {},
expression:{}
expression:{},
networkMode: "grn"
}));
});

Expand All @@ -368,7 +408,8 @@ describe("Import from SIF", function () {
negativeWeights: [],
sheetType: "weighted",
meta: {},
expression:{}
expression:{},
networkMode: "grn"
}));
});

Expand Down Expand Up @@ -399,11 +440,11 @@ describe("Import from SIF syntactic checker", function () {

it("should produce no warnings or errors for correct data", function () {
expect(
importController.sifToGrnsight(unweightedTestSif).errors.length
importController.sifToGrnsight(unweightedGRNTestSif).errors.length
).to.equal(0);
});

it("should throw an error for unweighted graphs with relationship types other than 'pd'", function () {
it("should throw an error for unweighted graphs with relationship types other than 'pd' and 'pp", function () {
expect(
importController.sifToGrnsight(unweightedTestSifWithIncorrectRelationshipType).errors.length
).to.equal(1);
Expand Down Expand Up @@ -470,7 +511,7 @@ describe("Import from SIF syntactic checker", function () {
it("should accept trivially tabbed workbooks", function () {
expect(
importController.sifToGrnsight(triviallyTabbedUnweightedWorkbook)
).to.deep.equal(expectedUnweightedWorkbook);
).to.deep.equal(expectedUnweightedGRNWorkbook);
});

});
11 changes: 8 additions & 3 deletions web-client/public/js/setup-load-and-import-handlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,15 @@ export const setupLoadAndImportHandlers = (grnState) => {
break;
}
}
// TODO 👆🏼 The back end will add a `networkType` property to this object.
// The web app can then adjust various things based on that.
grnState.workbook = workbook;
grnState.mode = workbook.meta.data.workbookType;

if (grnState.name.includes(".sif")) {
grnState.mode = workbook.networkMode;
} else if (grnState.name.includes(".graphml")) {
grnState.mode = "grn";
} else {
grnState.mode = workbook.meta.data.workbookType;
}
grnState.workbook.expressionNames = Object.keys(workbook.expression);
if (uploadRoute !== "upload") {
grnState.annotateLinks();
Expand Down