RFC84: Data Entries Removal (#46)

Add command to remove sample to python wrapper

Implment sample removal command

Remove sample from the tab delimited tables as well

Do samples removal in a transaction

Move removing sample everywhere in study to respective DAOs

To be able to reuse this functionality

Refactor sample removal test by using stream of

Test and fix edge cases of sample removal command

Add command to remove patient to python wrapper

Add java command to remove patient in a study

Increase test independence

Adding or removing profile/sample should break only one test

Fix patient id to sample ids mapping bug

Throw exception when sample with GSVA is removed

Throw exception when generic profile samples list is empty

Refactor patient and sample removal code

Remove unused logger and imports

Remove unused code from cna discrete long data tests

scripts/importer/cbioportalImporter.py

@@ -36,6 +36,8 @@
 from .cbioportal_common import IMPORT_STUDY_CLASS
 from .cbioportal_common import UPDATE_STUDY_STATUS_CLASS
 from .cbioportal_common import REMOVE_STUDY_CLASS
+from .cbioportal_common import REMOVE_SAMPLES_CLASS
+from .cbioportal_common import REMOVE_PATIENTS_CLASS
 from .cbioportal_common import IMPORT_CASE_LIST_CLASS
 from .cbioportal_common import ADD_CASE_LIST_CLASS
 from .cbioportal_common import VERSION_UTIL_CLASS
@@ -53,10 +55,12 @@
 IMPORT_CANCER_TYPE = "import-cancer-type"
 IMPORT_STUDY = "import-study"
 REMOVE_STUDY = "remove-study"
+REMOVE_SAMPLES = "remove-samples"
+REMOVE_PATIENTS = "remove-patients"
 IMPORT_STUDY_DATA = "import-study-data"
 IMPORT_CASE_LIST = "import-case-list"
 
-COMMANDS = [IMPORT_CANCER_TYPE, IMPORT_STUDY, REMOVE_STUDY, IMPORT_STUDY_DATA, IMPORT_CASE_LIST]
+COMMANDS = [IMPORT_CANCER_TYPE, IMPORT_STUDY, IMPORT_STUDY_DATA, IMPORT_CASE_LIST, REMOVE_STUDY, REMOVE_SAMPLES, REMOVE_PATIENTS]
 
 # ------------------------------------------------------------------------------
 # sub-routines
@@ -104,6 +108,24 @@ def remove_study_id(jvm_args, study_id):
     args.append("--noprogress") # don't report memory usage and % progress
     run_java(*args)
 
+def remove_samples(jvm_args, study_ids, sample_ids):
+    args = jvm_args.split(' ')
+    args.append(REMOVE_SAMPLES_CLASS)
+    args.append("--study_ids")
+    args.append(study_ids)
+    args.append("--sample_ids")
+    args.append(sample_ids)
+    run_java(*args)
+
+def remove_patients(jvm_args, study_ids, patient_ids):
+    args = jvm_args.split(' ')
+    args.append(REMOVE_PATIENTS_CLASS)
+    args.append("--study_ids")
+    args.append(study_ids)
+    args.append("--patient_ids")
+    args.append(patient_ids)
+    run_java(*args)
+
 def update_case_lists(jvm_args, meta_filename, case_lists_file_or_dir = None):
     args = jvm_args.split(' ')
     args.append(UPDATE_CASE_LIST_CLASS)
@@ -213,7 +235,7 @@ def process_case_lists(jvm_args, case_list_dir):
         if not (case_list.startswith('.') or case_list.endswith('~')):
             import_case_list(jvm_args, os.path.join(case_list_dir, case_list))
 
-def process_command(jvm_args, command, meta_filename, data_filename, study_ids, update_generic_assay_entity = None):
+def process_command(jvm_args, command, meta_filename, data_filename, study_ids, patient_ids, sample_ids, update_generic_assay_entity = None):
     if command == IMPORT_CANCER_TYPE:
         import_cancer_type(jvm_args, data_filename)
     elif command == IMPORT_STUDY:
@@ -227,6 +249,10 @@ def process_command(jvm_args, command, meta_filename, data_filename, study_ids,
                 remove_study_id(jvm_args, study_id)
         else:
             raise RuntimeError('Your command uses both -id and -meta. Please, use only one of the two parameters.')
+    elif command == REMOVE_SAMPLES:
+        remove_samples(jvm_args, study_ids, sample_ids)
+    elif command == REMOVE_PATIENTS:
+        remove_patients(jvm_args, study_ids, patient_ids)
     elif command == IMPORT_STUDY_DATA:
         import_data(jvm_args, meta_filename, data_filename, update_generic_assay_entity)
     elif command == IMPORT_CASE_LIST:
@@ -505,7 +531,7 @@ def usage():
                            '--command [%s] --study_directory <path to directory> '
                            '--meta_filename <path to metafile>'
                            '--data_filename <path to datafile>'
-                           '--study_ids <cancer study ids for remove-study command, comma separated>' % (COMMANDS)), file=OUTPUT_FILE)
+                           '--study_ids <cancer study ids for remove-study or remove-samples command, comma separated>' % (COMMANDS)), file=OUTPUT_FILE)
 
 def check_args(command):
     if command not in COMMANDS:
@@ -545,23 +571,32 @@ def interface(args=None):
     parent_parser = argparse.ArgumentParser(description='cBioPortal meta Importer')
     add_parser_args(parent_parser)
     parser = argparse.ArgumentParser()
+    allowed_commands_csv = ', '.join(COMMANDS)
     subparsers = parser.add_subparsers(title='subcommands', dest='subcommand',
-                          help='Command for import. Allowed commands: import-cancer-type, '
-                          'import-study, import-study-data, import-case-list or '
-                          'remove-study')
+                          help='Command for import. Allowed commands: ' + allowed_commands_csv)
     import_cancer_type = subparsers.add_parser('import-cancer-type', parents=[parent_parser], add_help=False)
     import_study = subparsers.add_parser('import-study', parents=[parent_parser], add_help=False)
     import_study_data = subparsers.add_parser('import-study-data', parents=[parent_parser], add_help=False)
     import_case_list = subparsers.add_parser('import-case-list', parents=[parent_parser], add_help=False)
     remove_study = subparsers.add_parser('remove-study', parents=[parent_parser], add_help=False)
-
     remove_study.add_argument('-id', '--study_ids', type=str, required=False,
                         help='Cancer Study IDs for `remove-study` command, comma separated')
-    parser.add_argument('-c', '--command', type=str, required=False, 
+
+    remove_samples = subparsers.add_parser('remove-samples', parents=[], add_help=True)
+    remove_samples.add_argument('--study_ids', type=str, required=True,
+                        help='Cancer Study ID(s) that contains sample(s). Comma separated, if multiple.')
+    remove_samples.add_argument('--sample_ids', type=str, required=True,
+                        help='Sample ID(s). Comma separated, if multiple.')
+
+    remove_patients = subparsers.add_parser('remove-patients', parents=[], add_help=True)
+    remove_patients.add_argument('--study_ids', type=str, required=True,
+                        help='Cancer Study ID(s) that contains sample(s). Comma separated, if multiple.')
+    remove_patients.add_argument('--patient_ids', type=str, required=True,
+                        help='Patient ID(s). Comma separated, if multiple.')
+
+    parser.add_argument('-c', '--command', type=str, required=False,
                         help='This argument is outdated. Please use the listed subcommands, without the -c flag. '
-                        'Command for import. Allowed commands: import-cancer-type, '
-                        'import-study, import-study-data, import-case-list or '
-                        'remove-study')
+                        'Command for import. Allowed commands: ' + allowed_commands_csv)
     add_parser_args(parser)
     parser.add_argument('-id', '--study_ids', type=str, required=False,
                         help='Cancer Study IDs for `remove-study` command, comma separated')
@@ -647,7 +682,15 @@ def main(args):
     else:
         check_args(args.command)
         check_files(args.meta_filename, args.data_filename)
-        process_command(jvm_args, args.command, args.meta_filename, args.data_filename, args.study_ids, args.update_generic_assay_entity)
+        process_command(
+            jvm_args,
+            args.command,
+            args.meta_filename,
+            args.data_filename,
+            args.study_ids,
+            args.patient_ids if hasattr(args, 'patient_ids') else None,
+            args.sample_ids if hasattr(args, 'sample_ids') else None,
+            args.update_generic_assay_entity)
 
 # ------------------------------------------------------------------------------
 # ready to roll

scripts/importer/cbioportal_common.py

@@ -34,6 +34,8 @@
 IMPORT_STUDY_CLASS = "org.mskcc.cbio.portal.scripts.ImportCancerStudy"
 UPDATE_STUDY_STATUS_CLASS = "org.mskcc.cbio.portal.scripts.UpdateCancerStudy"
 REMOVE_STUDY_CLASS = "org.mskcc.cbio.portal.scripts.RemoveCancerStudy"
+REMOVE_SAMPLES_CLASS = "org.mskcc.cbio.portal.scripts.RemoveSamples"
+REMOVE_PATIENTS_CLASS = "org.mskcc.cbio.portal.scripts.RemovePatients"
 IMPORT_CANCER_TYPE_CLASS = "org.mskcc.cbio.portal.scripts.ImportTypesOfCancers"
 IMPORT_CASE_LIST_CLASS = "org.mskcc.cbio.portal.scripts.ImportSampleList"
 ADD_CASE_LIST_CLASS = "org.mskcc.cbio.portal.scripts.AddCaseList"

src/main/java/org/mskcc/cbio/portal/dao/DaoGeneticAlteration.java

@@ -32,25 +32,30 @@
 
 package org.mskcc.cbio.portal.dao;
 
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import org.apache.commons.lang3.StringUtils;
 import org.mskcc.cbio.portal.model.CanonicalGene;
 
 import java.sql.Connection;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
 import java.util.Map.Entry;
-
-import com.fasterxml.jackson.databind.node.ObjectNode;
-
-import org.apache.commons.lang3.StringUtils;
+import java.util.Set;
 
 /**
  * Data Access Object for the Genetic Alteration Table.
  *
  * @author Ethan Cerami.
  */
 public class DaoGeneticAlteration {
+
     private static final String DELIM = ",";
     public static final String NAN = "NaN";
     private static DaoGeneticAlteration daoGeneticAlteration = null;

src/main/java/org/mskcc/cbio/portal/dao/DaoGeneticProfileSamples.java

@@ -64,7 +64,7 @@ public static int addGeneticProfileSamples(int geneticProfileId, List<Integer> o
         StringBuffer orderedSampleListBuf = new StringBuffer();
         //  Created Joined String, based on DELIM token
         for (Integer sampleId :  orderedSampleList) {
-            orderedSampleListBuf.append(Integer.toString(sampleId)).append(DELIM);
+            orderedSampleListBuf.append(sampleId).append(DELIM);
         }
         try {
             con = JdbcUtil.getDbConnection(DaoGeneticProfileSamples.class);
@@ -126,7 +126,11 @@ public static ArrayList <Integer> getOrderedSampleList(int geneticProfileId) thr
                 String orderedSampleList = rs.getString("ORDERED_SAMPLE_LIST");
 
                 //  Split, based on DELIM token
-                String parts[] = orderedSampleList.split(DELIM);
+                String[] parts = orderedSampleList.split(DELIM);
+                if (parts.length == 1 && parts[0].isBlank()) {
+                    throw new IllegalStateException("genetic_profile_samples row for geneticProfileId="
+                            + geneticProfileId + " has blank ORDERED_SAMPLE_LIST. Consider removing it.");
+                }
                 ArrayList <Integer> sampleList = new ArrayList <Integer>();
                 for (String internalSampleId : parts) {
                     sampleList.add(Integer.parseInt(internalSampleId));

src/main/java/org/mskcc/cbio/portal/dao/DaoPatient.java

@@ -35,9 +35,12 @@
 import org.mskcc.cbio.portal.model.*;
 
 import org.apache.commons.collections4.map.MultiKeyMap;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.sql.*;
 import java.util.*;
+import java.util.stream.Collectors;
 
 /**
  * DAO to `patient`.
@@ -46,6 +49,8 @@
  */
 public class DaoPatient {
 
+    private static final Logger log = LoggerFactory.getLogger(DaoPatient.class);
+
     private static final String SAMPLE_COUNT_ATTR_ID = "SAMPLE_COUNT";
 
     private static final Map<Integer, Patient> byInternalId = new HashMap<Integer, Patient>();
@@ -215,4 +220,58 @@ private static Patient extractPatient(ResultSet rs) throws SQLException
 			throw new SQLException(e);
 		}
     }
+
+    /**
+     * Removes patients information from the study
+     * @param internalStudyId - id of the study that contains the patients
+     * @param patientStableIds - patient stable ids to remove
+     * @throws DaoException
+     */
+    public static void deletePatients(int internalStudyId, Set<String> patientStableIds) throws DaoException
+    {
+        if (patientStableIds == null || patientStableIds.isEmpty()) {
+            log.info("No patients specified to remove for study with internal id={}. Skipping.", internalStudyId);
+            return;
+        }
+        log.info("Removing {} patients from study with internal id={} ...", patientStableIds, internalStudyId);
+
+        Set<Integer> internalPatientIds = findInternalPatientIdsInStudy(internalStudyId, patientStableIds);
+        Set<String> patientsSampleStableIds = internalPatientIds.stream().flatMap(internalPatientId ->
+                DaoSample.getSamplesByPatientId(internalPatientId).stream().map(Sample::getStableId))
+                .collect(Collectors.toSet());
+        DaoSample.deleteSamples(internalStudyId, patientsSampleStableIds);
+
+        Connection con = null;
+        PreparedStatement pstmt = null;
+        try {
+            con = JdbcUtil.getDbConnection(DaoPatient.class);
+            pstmt = con.prepareStatement("DELETE FROM `patient` WHERE `INTERNAL_ID` IN ("
+                    + String.join(",", Collections.nCopies(internalPatientIds.size(), "?"))
+                    + ")");
+            int parameterIndex = 1;
+            for (Integer internalPatientId : internalPatientIds) {
+                pstmt.setInt(parameterIndex++, internalPatientId);
+            };
+            pstmt.executeUpdate();
+        }
+        catch (SQLException e) {
+            throw new DaoException(e);
+        }
+        finally {
+            JdbcUtil.closeAll(DaoPatient.class, con, pstmt, null);
+        }
+        log.info("Removing {} patients from study with internal id={} done.", patientStableIds, internalStudyId);
+    }
+
+    public static Set<Integer> findInternalPatientIdsInStudy(Integer internalStudyId, Set<String> patientStableIds) {
+        HashSet<Integer> internalPatientIds = new HashSet<>();
+        for (String patientId : patientStableIds) {
+            Patient patientByCancerStudyAndPatientId = DaoPatient.getPatientByCancerStudyAndPatientId(internalStudyId, patientId);
+            if (patientByCancerStudyAndPatientId == null) {
+                throw new NoSuchElementException("Patient with stable id=" + patientId + " not found in study with internal id=" + internalStudyId + ".");
+            }
+            internalPatientIds.add(patientByCancerStudyAndPatientId.getInternalId());
+        }
+        return internalPatientIds;
+    }
 }