Repository: wireservice/csvkit
Issue: Change -l to use "original" line numbers for csvgrep ?

I was surprised by the behavior of csvgrep -l -- the lines are numbered from 1 to N consecutively based on the post-grep output. I was kind of expecting to see the original line numbers.

This would be a backwards-incompatible change, in a sense, so not sure how you want to handle it.


Changed files:
csvkit/utilities/csvsort.py, csvkit/utilities/csvstat.py, tests/test_cli.py, csvkit/cli.py, csvkit/table.py, csvkit/utilities/csvgrep.py, tests/test_utilities/test_csvgrep.py, csvkit/utilities/csvcut.py

diff --git a/csvkit/cli.py b/csvkit/cli.py
index d74cb91..60ffc53 100644
--- a/csvkit/cli.py
+++ b/csvkit/cli.py
@@ -156,7 +156,7 @@ class CSVKitUtility(object):
             self.argparser.add_argument('-e', '--encoding', dest='encoding', default='utf-8',
                                         help='Specify the encoding the input CSV file.')
         if 'S' not in self.override_flags:
-            self.argparser.add_argument('-S', '--skipinitialspace', dest='skipinitialspace', default=False, action='store_true',
+            self.argparser.add_argument('-S', '--skipinitialspace', dest='skipinitialspace', action='store_true',
                                         help='Ignore whitespace immediately following the delimiter.')
         if 'H' not in self.override_flags:
             self.argparser.add_argument('-H', '--no-header-row', dest='no_header_row', action='store_true',
@@ -255,8 +255,14 @@ class CSVKitUtility(object):
 
         sys.excepthook = handler
 
-    def get_rows_and_column_names_and_column_ids(self):
-        rows = agate.reader(self.input_file, **self.reader_kwargs)
+    def get_column_offset(self):
+        if self.args.zero_based:
+            return 0
+        else:
+            return 1
+
+    def get_rows_and_column_names_and_column_ids(self, **kwargs):
+        rows = agate.reader(self.input_file, **kwargs)
 
         if self.args.no_header_row:
             # Peek at a row to get the number of columns.
@@ -266,7 +272,11 @@ class CSVKitUtility(object):
         else:
             column_names = next(rows)
 
-        column_ids = parse_column_identifiers(self.args.columns, column_names, self.args.zero_based, getattr(self.args, 'not_columns', None))
+        column_offset = self.get_column_offset()
+        if self.args.line_numbers:
+            column_offset -= 1
+
+        column_ids = parse_column_identifiers(self.args.columns, column_names, column_offset, getattr(self.args, 'not_columns', None))
 
         return rows, column_names, column_ids
 
@@ -301,7 +311,7 @@ def make_default_headers(n):
     return tuple(agate.utils.letter_name(i) for i in range(n))
 
 
-def match_column_identifier(column_names, c, zero_based=False):
+def match_column_identifier(column_names, c, column_offset=1):
     """
     Determine what column a single column id (name or index) matches in a series of column names.
     Note that integer values are *always* treated as positional identifiers. If you happen to have
@@ -311,9 +321,7 @@ def match_column_identifier(column_names, c, zero_based=False):
         return column_names.index(c)
     else:
         try:
-            c = int(c)
-            if not zero_based:
-                c -= 1
+            c = int(c) - column_offset
         # Fail out if neither a column name nor an integer
         except:
             raise ColumnIdentifierError("Column '%s' is invalid. It is neither an integer nor a column name. Column names are: %s" % (c, repr(column_names)[1:-1]))
@@ -329,7 +337,7 @@ def match_column_identifier(column_names, c, zero_based=False):
     return c
 
 
-def parse_column_identifiers(ids, column_names, zero_based=False, excluded_columns=None):
+def parse_column_identifiers(ids, column_names, column_offset=1, excluded_columns=None):
     """
     Parse a comma-separated list of column indices AND/OR names into a list of integer indices.
     Ranges of integers can be specified with two integers separated by a '-' or ':' character. Ranges of
@@ -347,7 +355,7 @@ def parse_column_identifiers(ids, column_names, zero_based=False, excluded_colum
 
         for c in ids.split(','):
             try:
-                columns.append(match_column_identifier(column_names, c, zero_based))
+                columns.append(match_column_identifier(column_names, c, column_offset))
             except ColumnIdentifierError:
                 if ':' in c:
                     a, b = c.split(':', 1)
@@ -370,7 +378,7 @@ def parse_column_identifiers(ids, column_names, zero_based=False, excluded_colum
                     raise ColumnIdentifierError("Invalid range %s. Ranges must be two integers separated by a - or : character.")
 
                 for x in range(a, b):
-                    columns.append(match_column_identifier(column_names, x, zero_based))
+                    columns.append(match_column_identifier(column_names, x, column_offset))
     else:
         columns = range(len(column_names))
 
@@ -379,7 +387,7 @@ def parse_column_identifiers(ids, column_names, zero_based=False, excluded_colum
     if excluded_columns:
         for c in excluded_columns.split(','):
             try:
-                excludes.append(match_column_identifier(column_names, c, zero_based))
+                excludes.append(match_column_identifier(column_names, c, column_offset))
             except ColumnIdentifierError:
                 if ':' in c:
                     a, b = c.split(':', 1)
@@ -402,6 +410,6 @@ def parse_column_identifiers(ids, column_names, zero_based=False, excluded_colum
                     raise ColumnIdentifierError("Invalid range %s. Ranges must be two integers separated by a - or : character.")
 
                 for x in range(a, b):
-                    excludes.append(match_column_identifier(column_names, x, zero_based))
+                    excludes.append(match_column_identifier(column_names, x, column_offset))
 
     return [c for c in columns if c not in excludes]
diff --git a/csvkit/table.py b/csvkit/table.py
index 1072bf8..b111c79 100644
--- a/csvkit/table.py
+++ b/csvkit/table.py
@@ -130,7 +130,7 @@ class Table(list):
         return 0
 
     @classmethod
-    def from_csv(cls, f, name='from_csv_table', sniff_limit=None, column_ids=None, blanks_as_nulls=True, zero_based=False, infer_types=True, no_header_row=False, **kwargs):
+    def from_csv(cls, f, name='from_csv_table', sniff_limit=None, column_ids=None, blanks_as_nulls=True, column_offset=1, infer_types=True, no_header_row=False, **kwargs):
         """
         Creates a new Table from a file-like object containing CSV data.
 
@@ -166,7 +166,7 @@ class Table(list):
             pass
 
         if no_header_row or column_ids:
-            column_ids = parse_column_identifiers(column_ids, headers, zero_based)
+            column_ids = parse_column_identifiers(column_ids, headers, column_offset)
             headers = [headers[c] for c in column_ids]
         else:
             column_ids = range(len(headers))
diff --git a/csvkit/utilities/csvcut.py b/csvkit/utilities/csvcut.py
index 79f9a44..06a7188 100644
--- a/csvkit/utilities/csvcut.py
+++ b/csvkit/utilities/csvcut.py
@@ -32,7 +32,7 @@ class CSVCut(CSVKitUtility):
             self.print_column_names()
             return
 
-        rows, column_names, column_ids = self.get_rows_and_column_names_and_column_ids()
+        rows, column_names, column_ids = self.get_rows_and_column_names_and_column_ids(**self.reader_kwargs)
 
         output = agate.writer(self.output_file, **self.writer_kwargs)
         output.writerow([column_names[column_id] for column_id in column_ids])
diff --git a/csvkit/utilities/csvgrep.py b/csvkit/utilities/csvgrep.py
index ea02df0..c8d7ac3 100644
--- a/csvkit/utilities/csvgrep.py
+++ b/csvkit/utilities/csvgrep.py
@@ -37,7 +37,12 @@ class CSVGrep(CSVKitUtility):
         if self.args.regex is None and self.args.pattern is None and self.args.matchfile is None:
             self.argparser.error('One of -r, -m or -f must be specified, unless using the -n option.')
 
-        rows, column_names, column_ids = self.get_rows_and_column_names_and_column_ids()
+        reader_kwargs = self.reader_kwargs
+        writer_kwargs = self.writer_kwargs
+        if writer_kwargs.pop('line_numbers', False):
+            reader_kwargs = {'line_numbers': True}
+
+        rows, column_names, column_ids = self.get_rows_and_column_names_and_column_ids(**reader_kwargs)
 
         if self.args.regex:
             pattern = re.compile(self.args.regex)
@@ -50,7 +55,7 @@ class CSVGrep(CSVKitUtility):
         patterns = dict((column_id, pattern) for column_id in column_ids)
         filter_reader = FilteringCSVReader(rows, header=False, patterns=patterns, inverse=self.args.inverse)
 
-        output = agate.writer(self.output_file, **self.writer_kwargs)
+        output = agate.writer(self.output_file, **writer_kwargs)
         output.writerow(column_names)
 
         for row in filter_reader:
diff --git a/csvkit/utilities/csvsort.py b/csvkit/utilities/csvsort.py
index 87c7ed4..56e202f 100644
--- a/csvkit/utilities/csvsort.py
+++ b/csvkit/utilities/csvsort.py
@@ -31,7 +31,7 @@ class CSVSort(CSVKitUtility):
             column_types = None
 
         table = agate.Table.from_csv(self.input_file, sniff_limit=self.args.sniff_limit, header=not self.args.no_header_row, column_types=column_types, **self.reader_kwargs)
-        column_ids = parse_column_identifiers(self.args.columns, table.column_names, self.args.zero_based)
+        column_ids = parse_column_identifiers(self.args.columns, table.column_names, column_offset=self.get_column_offset())
         table = table.order_by(lambda row: [(row[column_id] is not None, row[column_id]) for column_id in column_ids], reverse=self.args.reverse)
         table.to_csv(self.output_file, **self.writer_kwargs)
 
diff --git a/csvkit/utilities/csvstat.py b/csvkit/utilities/csvstat.py
index f5fcb8c..d4feb2b 100644
--- a/csvkit/utilities/csvstat.py
+++ b/csvkit/utilities/csvstat.py
@@ -79,7 +79,7 @@ class CSVStat(CSVKitUtility):
             self.input_file,
             sniff_limit=self.args.sniff_limit,
             column_ids=self.args.columns,
-            zero_based=self.args.zero_based,
+            column_offset=self.get_column_offset(),
             no_header_row=self.args.no_header_row,
             **self.reader_kwargs
         )
diff --git a/tests/test_cli.py b/tests/test_cli.py
index 9381608..a78614b 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -15,29 +15,29 @@ class TestCli(unittest.TestCase):
 
     def test_match_column_identifier_string(self):
         self.assertEqual(2, match_column_identifier(self.headers, 'i_work_here'))
-        self.assertEqual(2, match_column_identifier(self.headers, 'i_work_here', zero_based=True))
+        self.assertEqual(2, match_column_identifier(self.headers, 'i_work_here', column_offset=0))
 
     def test_match_column_identifier_numeric(self):
         self.assertEqual(2, match_column_identifier(self.headers, 3))
-        self.assertEqual(3, match_column_identifier(self.headers, 3, zero_based=True))
+        self.assertEqual(3, match_column_identifier(self.headers, 3, column_offset=0))
 
     def test_match_column_which_could_be_integer_name_is_treated_as_positional_id(self):
         self.assertEqual(0, match_column_identifier(self.headers, '1'))
-        self.assertEqual(1, match_column_identifier(self.headers, '1', zero_based=True))
+        self.assertEqual(1, match_column_identifier(self.headers, '1', column_offset=0))
 
     def test_parse_column_identifiers(self):
         self.assertEqual([2, 0, 1], parse_column_identifiers('i_work_here,1,name', self.headers))
-        self.assertEqual([2, 1, 1], parse_column_identifiers('i_work_here,1,name', self.headers, zero_based=True))
+        self.assertEqual([2, 1, 1], parse_column_identifiers('i_work_here,1,name', self.headers, column_offset=0))
 
     def test_range_notation(self):
         self.assertEqual([0, 1, 2], parse_column_identifiers('1:3', self.headers))
-        self.assertEqual([1, 2, 3], parse_column_identifiers('1:3', self.headers, zero_based=True))
+        self.assertEqual([1, 2, 3], parse_column_identifiers('1:3', self.headers, column_offset=0))
         self.assertEqual([1, 2, 3], parse_column_identifiers('2-4', self.headers))
-        self.assertEqual([2, 3, 4], parse_column_identifiers('2-4', self.headers, zero_based=True))
+        self.assertEqual([2, 3, 4], parse_column_identifiers('2-4', self.headers, column_offset=0))
         self.assertEqual([0, 1, 2, 3], parse_column_identifiers('1,2:4', self.headers))
-        self.assertEqual([1, 2, 3, 4], parse_column_identifiers('1,2:4', self.headers, zero_based=True))
+        self.assertEqual([1, 2, 3, 4], parse_column_identifiers('1,2:4', self.headers, column_offset=0))
         self.assertEqual([4, 2, 5], parse_column_identifiers('more-header-values,3,stuff', self.headers))
-        self.assertEqual([4, 3, 5], parse_column_identifiers('more-header-values,3,stuff', self.headers, zero_based=True))
+        self.assertEqual([4, 3, 5], parse_column_identifiers('more-header-values,3,stuff', self.headers, column_offset=0))
 
     def test_range_notation_open_ended(self):
         self.assertEqual([0, 1, 2], parse_column_identifiers(':3', self.headers))
diff --git a/tests/test_utilities/test_csvgrep.py b/tests/test_utilities/test_csvgrep.py
index 17d396e..be4e396 100644
--- a/tests/test_utilities/test_csvgrep.py
+++ b/tests/test_utilities/test_csvgrep.py
@@ -48,3 +48,9 @@ class TestCSVGrep(CSVKitTestCase, ColumnsTests, EmptyFileTests, NamesTests):
             ['State Name', 'State Abbreviate', 'Code', 'Montgomery GI Bill-Active Duty', 'Montgomery GI Bill- Selective Reserve', 'Dependents\\' Educational Assistance', 'Reserve Educational Assistance Program', 'Post-Vietnam Era Veteran\\'s Educational Assistance Program', 'TOTAL', ''],
             ['ILLINOIS', 'IL', '17', '15,659', '2,491', '2,025', '1,770', '19', '21,964', ''],
         ])
+
+    def test_match_with_line_numbers(self):
+        self.assertRows(['-c', '1', '-m', 'ILLINOIS', '-l', 'examples/realdata/FY09_EDU_Recipients_by_State.csv'], [
+            ['line_numbers', 'State Name', 'State Abbreviate', 'Code', 'Montgomery GI Bill-Active Duty', 'Montgomery GI Bill- Selective Reserve', 'Dependents\\' Educational Assistance', 'Reserve Educational Assistance Program', 'Post-Vietnam Era Veteran\\'s Educational Assistance Program', 'TOTAL', ''],
+            ['14', 'ILLINOIS', 'IL', '17', '15,659', '2,491', '2,025', '1,770', '19', '21,964', ''],
+        ])