Add a showtable script by saimn · Pull Request #6859 · astropy/astropy

saimn · 2017-11-15T21:45:18Z

Closes #3022. I often miss such a script and there was an agreement in #3022. Basically it is just a wrapper around Table.read and Table.pprint so it supports printing ascii, fits, votable and hdf5. I also added options specific to each format (see the script help below), suggestions welcome if you think some option is missing.

❯ showtable --help
usage: showtable [-h] [--max-lines MAX_LINES] [--max-width MAX_WIDTH]
                 [--hide-unit] [--show-dtype] [--format FORMAT] [--hdu HDU]
                 [--path PATH] [--table_id TABLE_ID]
                 filename [filename ...]

Print tables from ASCII, FITS, HDF5, VOTable file(s).

positional arguments:
  filename              Path to one or more files.

optional arguments:
  -h, --help            show this help message and exit
  --max-lines MAX_LINES
                        Maximum number of lines in table output.
  --max-width MAX_WIDTH
                        Maximum character width of output.
  --hide-unit           Hide the header row for unit (which is shown only if
                        one or more columns has a unit).
  --show-dtype          Include a header row for column dtypes.
  --format FORMAT       Input table format (only for ASCII files).
  --hdu HDU             Name of the HDU to show (only for FITS files).
  --path PATH           The path from which to read the table (only for HDF5
                        files).
  --table_id TABLE_ID   The table to read in (only for VOTable files).

astropy-bot · 2017-11-15T21:45:20Z

Hi there @saimn 👋 - thanks for the pull request! I'm just a friendly 🤖 that checks for issues related to the changelog and making sure that this pull request is milestoned and labeled correctly. This is mainly intended for the maintainers, so if you are not a maintainer you can ignore this, and a maintainer will let you know if any action is required on your part 😃.

Everything looks good from my point of view! 👍

If there are any issues with this message, please report them here.

saimn · 2017-11-15T21:47:41Z

Missing:

changelog entry
mention in docs

Note that I created the file inside a new directory astropy/table/scripts/showtable.py as it looks logical but I can move it if needed.

pllim · 2017-11-15T21:51:26Z

c/c @SaraOgaz in case this is of interest for STAK.

astrofrog · 2017-11-15T23:07:58Z

+    try:
+        table = Table.read(filename, **read_kwargs)
+        table.pprint(max_lines=args.max_lines, max_width=args.max_width,
+                     show_unit=not args.hide_unit, show_dtype=args.show_dtype)


What about using table.more? Or maybe we could have a flag to switch between 'cat' mode (pprint) and 'more' mode?

I never use Table.more so I had a look and yes it could useful to have an option. I prefer the "cat mode " as default as personally I'm used to pipe the output to less (or more) which allows to search (not possible with Table.more).

taldcroft

Great idea @saimn !

taldcroft · 2017-11-16T16:00:32Z

+def main(args=None):
+    """The main function called by the `showtable` script."""
+    parser = argparse.ArgumentParser(
+        description=('Print tables from ASCII, FITS, HDF5, VOTable file(s).'))


I'd suggest putting some description here of the default output width / lines and how to control it. I think the fact that long/wide tables get clipped can be initially confusing. Something along the lines of:

The default behavior is make the table output fit onto a single screen page. For a long and wide table this will mean cutting out inner rows and columns. To print **all** the rows or columns use ``--max-lines=-1`` or ``max-width=-1``, respectively.

taldcroft · 2017-11-16T16:02:30Z

+    addarg('--more', action='store_true',
+           help='Use the pager mode from Table.more.')
+    addarg('--max-lines', type=int,
+           help='Maximum number of lines in table output.')


Suggest help='Max number of lines in table output (default=screen length, -1 for no limit).'

Same idea for --max-width. (I replaced "Maximum" with "Max" so it will format slightly better, but maybe that's not necessary.)

Thanks for the suggestions, I have done the changes (using Maximum as it will use 2 lines anyway).

taldcroft · 2017-11-16T16:31:56Z

+           help='Include a header row for column dtypes.')
+
+    # ASCII-specific arguments
+    # FIXME: add more args ? (delimiter, guess ?)


I wonder if it would make sense to generalize this so that all the reader-specific kwargs are not initially pre-defined in the argparse parser. See e.g. for a hackish way to do this (which BTW doesn't totally work...)
https://stackoverflow.com/questions/37367331/is-it-possible-to-use-argparse-to-capture-an-arbitrary-set-of-optional-arguments

Probably a better option is to pre-code allowed options but use subparsers to keep them tidy and compartmentalized. E.g.

showtable ascii filename.dat --delimiter='|' --quotechar='"' --data-start=5 showtable ascii --help . # shows ascii subparser options

I'm not sure it's useful to add all possible parameters, my idea was to add only the most frequent ones (delimiter is certainly useful). The issue with subparsers is that it prevents a more generic use where you don't have to know the file type in advance, Table.read is able to guess many formats. Also having to specify always a subparser is cumbersome !

taldcroft · 2017-11-16T16:35:09Z

Also, you could probably use this interface to print the results of table.info (with an --info flag?).

saimn · 2017-11-17T22:19:57Z

Updated --help output with the new parameters (--delimiter, --info and --stats), I think it's still reasonably short:

❯ showtable --help
usage: showtable [-h] [--more] [--info] [--stats] [--max-lines MAX_LINES]
                 [--max-width MAX_WIDTH] [--hide-unit] [--show-dtype]
                 [--format FORMAT] [--delimiter DELIMITER] [--hdu HDU]
                 [--path PATH] [--table_id TABLE_ID]
                 filename [filename ...]

Print tables from ASCII, FITS, HDF5, VOTable file(s).The default behavior is
make the table output fit onto a single screen page. For a long and wide table
this will mean cutting out inner rows and columns. To print **all** the rows
or columns use ``--max-lines=-1`` or ``max-width=-1``, respectively.

positional arguments:
  filename              Path to one or more files.

optional arguments:
  -h, --help            show this help message and exit
  --more                Use the pager mode from Table.more.
  --info                Show information about the table columns.
  --stats               Show statistics about the table columns.
  --max-lines MAX_LINES
                        Maximum number of lines in table output
                        (default=screen length, -1 for no limit).
  --max-width MAX_WIDTH
                        Maximum width in table output (default=screen width,
                        -1 for no limit).
  --hide-unit           Hide the header row for unit (which is shown only if
                        one or more columns has a unit).
  --show-dtype          Include a header row for column dtypes.
  --format FORMAT       Input table format (only for ASCII files).
  --delimiter DELIMITER
                        Column delimiter string (only for ASCII files).
  --hdu HDU             Name of the HDU to show (only for FITS files).
  --path PATH           The path from which to read the table (only for HDF5
                        files).
  --table_id TABLE_ID   The table to read in (only for VOTable files).

saimn · 2017-11-17T22:49:32Z

I have also added a mention of the script in the unified I/O page :

taldcroft · 2017-11-18T00:13:17Z

@saimn - About the subparser, or else generalized support for sub-format options, I think you need to think about this as a high-visibility script that will be around for 15 or 20 years.

Pretty quickly we will add support for reading ASDF files, so that will result in at least one (if not more) new params. And with the mem-mapping in FITS it would make sense to add support in the API for reading in only some of the columns. Then a user will want the astropy_native option, and another will say that they have an ASCII file with % for comments, and another will say that they have a file that starts on the 5th line, and so on.

So I think that it is worth thinking hard about this script and future-proofing the UI, even if it means typing showtable fits filename.fits instead of showtable filename.fits. Or maybe there is another option (subformat required only to specify subformat options?), but I think that having a flat set of options that handles every file reader for the next 15-20 years will get messy.

saimn · 2017-11-20T23:00:40Z

@taldcroft , your arguments make perfectly sense, (and I agree it's good to think about this now before the is widely used), despite that I would prefer to avoid subparsers. I think subparsers makes the command more complicated than what it should be, you can read a table with Table.read('foo.fits') or Table.read('foo.csv'), it should be as simple for the script.
But that's just my own preference, I can live with subparsers if I really have to 😉, what do others think ?

taldcroft · 2017-11-21T14:00:56Z

@saimn - agreed that we need to find a balance between overdesigning and getting into a mess down the road. I too am interested in other opinions.

If you put in a general mechanism for handling any kwargs that are not parsed by argparse, then you get back to the simplicity but full generality of Table.read(). What you lose is documentation of available kwargs, but this is a long-standing problem with Table.read() as well so it would be OK.

saimn · 2017-11-27T21:45:56Z

I have used argument groups to group arguments for each filetype, which gives a much better output I think (see below) and would allow to add a few more arguments while not being too messy.

What do you think, compared to the sub-parsers option mentioned above, @taldcroft (and others - @astrofrog , @pllim , ... ? 😉 )

❯ showtable --help
usage: showtable [-h] [--more] [--info] [--stats] [--max-lines MAX_LINES]
                 [--max-width MAX_WIDTH] [--hide-unit] [--show-dtype]
                 [--format FORMAT] [--delimiter DELIMITER] [--hdu HDU]
                 [--path PATH] [--table-id TABLE_ID]
                 filename [filename ...]

Print tables from ASCII, FITS, HDF5, VOTable file(s). The tables are read with
'astropy.table.Table.read' and are printed with 'astropy.table.Table.pprint'.
The default behavior is to make the table output fit onto a single screen
page. For a long and wide table this will mean cutting out inner rows and
columns. To print **all** the rows or columns use ``--max-lines=-1`` or ``max-
width=-1``, respectively.

positional arguments:
  filename              path to one or more files

optional arguments:
  -h, --help            show this help message and exit
  --more                use the pager mode from Table.more
  --info                show information about the table columns
  --stats               show statistics about the table columns

pprint arguments:
  --max-lines MAX_LINES
                        maximum number of lines in table output
                        (default=screen length, -1 for no limit)
  --max-width MAX_WIDTH
                        maximum width in table output (default=screen width,
                        -1 for no limit)
  --hide-unit           hide the header row for unit (which is shown only if
                        one or more columns has a unit)
  --show-dtype          include a header row for column dtypes

ASCII arguments:
  --format FORMAT       input table format
  --delimiter DELIMITER
                        column delimiter string

FITS arguments:
  --hdu HDU             name of the HDU to show

HDF5 arguments:
  --path PATH           the path from which to read the table

VOTable arguments:
  --table-id TABLE_ID   the table to read in

taldcroft · 2017-11-27T22:36:38Z

Ah, I didn't know about argument groups. That works for me and looks nice.

I wonder if --format should be global in case some file fails to be auto-identified. E.g. a FITS file that doesn't have exactly the initial signature and an extension that doesn't match.

saimn · 2017-11-27T22:50:15Z

@taldcroft - Is it possible to force the Reader class used by the registry with a 'format' keyword ?

Also there is a test error with end of lines on appveyor, with Table.info's output. Why is it different from Table.pprint ?

pllim · 2017-11-27T22:54:49Z

@saimn , I like the grouping -- very nice! 👍

taldcroft · 2017-11-28T15:56:53Z

Is it possible to force the Reader class used by the registry with a 'format' keyword ?

Are you talking about the Reader class in io.ascii or in the general unified I/O scheme? For io.ascii the answer is yes, via the format keyword which takes the "nice" ASCII representation of the Reader class name (with the caveat that it is prepended by 'ascii.' in unified I/O). In the latter the reader class is determined from the caller, i.e. Table.read and this cannot be overridden (AFAIK).

saimn · 2017-11-28T16:01:39Z

I mean for the general case, if I have an exotic fits extension is it possible to use format='fits' ? I also think it is not possible currently, but wasn't sure given your sentence above - ("I wonder if --format should be global in case some file fails to be auto-identified. E.g. a FITS file...")

saimn · 2017-11-28T16:04:58Z

Also the appveyor issue is fixed, so all tests pass, and argument groups seems to have the consensus. Anything else that should be added/modified ?

taldcroft

More changes, but now I've looked through everything in some detail.

taldcroft · 2017-11-28T16:58:02Z


 - Allowed to remove table rows through the ``__delitem__`` method. [#5839]

+- Added a new ``showtable`` command-line script to view binary tables. [#6859]


to view binary or ASCII table files.

taldcroft · 2017-11-28T17:03:44Z

+    # ASCII-specific arguments
+    ascii_args = parser.add_argument_group('ASCII arguments')
+    addarg = ascii_args.add_argument
+    addarg('--format', help='input table format')


This should be moved out of the ASCII-specific args, since indeed one can specify format='fits' or 'hdf5' or 'ascii.ipac' etc.

taldcroft · 2017-11-28T17:07:40Z

+            table output fit onto a single screen page.  For a long and wide
+            table this will mean cutting out inner rows and columns.  To print
+            **all** the rows or columns use ``--max-lines=-1`` or
+            ``max-width=-1``, respectively.


It would be useful for users to have a link to the complete list of available format values: http://astropy.readthedocs.io/en/latest/io/unified.html#built-in-table-readers-writers

taldcroft · 2017-11-28T18:01:30Z

+        if args.info:
+            print(table.info)
+        elif args.stats:
+            table.info('stats')


For consistency you can write:

if args.info: table.info('attributes') elif args.stats: table.info('stats') else: ...

taldcroft · 2017-11-28T18:06:54Z

+def test_info(capsys):
+    showtable.main([os.path.join(FITS_ROOT, 'data/table.fits'), '--info'])
+    out, err = capsys.readouterr()
+    assert out == ('<Table length=3>{0}'


For reference one can do out.splitlines() == ['line 1', 'line2', etc..] to get around the OS-dependence. This is a bit easier for writing tests because you can write failing tests, run in the debugger, and then print out.splitlines() to get the right answer to paste into code (after verifying by hand that it seems right!).

Yep, it's probably more handy for debugging, I will change.

taldcroft · 2017-11-28T18:08:40Z

+def test_fits(capsys):
+    showtable.main([os.path.join(FITS_ROOT, 'data/table.fits')])
+    out, err = capsys.readouterr()
+    assert out == (' target V_mag\n'


I see tests are passing, but why don't these need the os.linesep fix? Is this a bug in Table.pprint that it isn't using os.linesep?

The behavior differs (pprint just prints each line, whereas info writes to stdout using os.linesep explicitly) but I don't know what behavior is expected for terminal output (as far as I know the convention differs for files ?).

taldcroft · 2017-11-28T18:14:04Z

+    NGC1002  12.3
+    NGC1003  15.2
+
+


Add something like

To get full documentation on the usage and available options do ``showtable --help``.

taldcroft · 2017-11-28T18:18:41Z

@@ -0,0 +1,112 @@
+# -*- coding: utf-8 -*-
+


On testing, I don't see tests for -max-lines, -hide-unit, and -show-dtype. Note that max_lines cannot be less than 7, and it applies to the header lines as well.

taldcroft · 2017-11-28T18:25:06Z

+        else:
+            formatter = table.more if args.more else table.pprint
+            formatter(max_lines=args.max_lines, max_width=args.max_width,
+                      show_unit=not args.hide_unit, show_dtype=args.show_dtype)


The show_unit arg is actually a 3-state argument:

True: always put in a line for the unit

False: never put in a line for the unit

None (default): put in a line for the unit if any of the columns has a unit

What is probably wanted here for default is None, so you don't get those blank lines in many of the typical cases (like in most of the tests). So show_unit=(False if args.hide_unit else None). This doesn't allow for forcing a line for units no matter what, but I think that is fine.

Indeed, I wanted None as default, thanks.

saimn · 2017-12-08T12:18:42Z

And now circleci fails because pytest-astropy requires pytest-mpl which installs matplotlib from source, oups! ping @drdavella (I think you already mentioned this issue somewhere?)

saimn · 2017-12-08T23:53:28Z

Tests are passing now, good for you @taldcroft ?

taldcroft · 2017-12-09T12:15:47Z

Thanks @saimn !

mhvk · 2017-12-10T00:40:38Z

This breaks my work on numpy 1.14 style changes (#6959), since it explicitly tests output - which depends on numpy version, with no allowance for floats being Are such tests this really needed? Obviously I can make the tests numpy-version dependent, but it is not for nothing that in our doctests we use +FLOAT_CMP. Could these tests use something similar?

p.s. It took me a while to figure out why a test_showtable.py was giving errors when I couldn't find it in my branch!

taldcroft · 2017-12-10T12:06:13Z

See #6959 (comment) (which says):

I think this is a case where we should be testing non-nice floating point values. The whole point of showtable is creating human-readable output that is informative and "pleasant". So using real-world values is important to catch formatting issues that our users will end up seeing.

SaOgaz · 2018-01-26T22:39:14Z

Hey @taldcroft, shouldn't we add this to the http://docs.astropy.org/en/stable/io/fits/usage/scripts.html doc page?

pllim · 2018-01-26T22:43:33Z

Minor correction: http://docs.astropy.org/en/latest/io/fits/usage/scripts.html (but I don't see it there either)

saimn · 2018-01-26T22:56:24Z

This page is for io.fits, but showtable is more general, hence the documentation is in the Unified I/O page: http://docs.astropy.org/en/latest/io/unified.html?highlight=showtable#command-line-utility
Maybe there should be a top-level page with all scripts instead ?

taldcroft · 2018-01-27T14:14:46Z

👍 on adding to the FITS page for scripts, since it is definitely applicable there. Unlike with code, I think it's generally OK to repeat yourself in docs to help with discoverability. (Of course there is an issue of maintaining all the docs to be accurate, but these scripts don't change that much.)

I agree that having a top-level page with all scripts would be a fine thing as well.

saimn added io.ascii io.fits io.misc io.votable unified-io labels Nov 15, 2017

saimn added this to the v3.0.0 milestone Nov 15, 2017

pllim added the Work in progress label Nov 15, 2017

astrofrog reviewed Nov 15, 2017

View reviewed changes

taldcroft requested changes Nov 16, 2017

View reviewed changes

saimn removed the Work in progress label Nov 17, 2017

saimn force-pushed the showtable branch from 4c14d81 to c82e15a Compare November 27, 2017 21:40

saimn force-pushed the showtable branch from 241a78c to ff52862 Compare November 27, 2017 23:30

bsipocz added the whatsnew-needed label Nov 28, 2017

taldcroft requested changes Nov 28, 2017

View reviewed changes

saimn added 13 commits December 8, 2017 13:07

Add __init__ to make ci happy (hopefully)

9a367a0

Add --more option to use Table.more

af73788

Enhance parameters help

8bdf19c

Add --delimiter for ascii formats

3cff087

Add --info and --stats

ffb8515

Add mention of the script in the docs

50767da

Use argument groups for filetype specific arguments

76212fe

Fix changelog after rebase

4cc9790

Fix .stats and .info tests on appveyor

f8eb84c

Address review comments

38a9811

Use splitlines to compare outputs

dddcaa6

Add tests for -max-lines, -hide-unit, and -show-dtype

12930fa

Fix tests with dtype following rebase

50338e5

saimn force-pushed the showtable branch from c054002 to 50338e5 Compare December 8, 2017 12:08

saimn added the Ready-for-final-review label Dec 8, 2017

taldcroft approved these changes Dec 9, 2017

View reviewed changes

taldcroft merged commit 33298a1 into astropy:master Dec 9, 2017

mhvk mentioned this pull request Dec 10, 2017

Numpy 1.14 style changes #6959

Merged

8 tasks

saimn deleted the showtable branch December 10, 2017 11:16

saimn mentioned this pull request Jan 30, 2018

Update test_showtable to split lines as in other tests #7134

Merged

pllim mentioned this pull request Feb 23, 2018

Rename showtable? #7233

Closed


		- Allowed to remove table rows through the ``__delitem__`` method. [#5839]

		- Added a new ``showtable`` command-line script to view binary tables. [#6859]

Uh oh!

Uh oh!

Conversation

saimn commented Nov 15, 2017

Uh oh!

astropy-bot Bot commented Nov 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

saimn commented Nov 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pllim commented Nov 15, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taldcroft left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taldcroft commented Nov 16, 2017

Uh oh!

saimn commented Nov 17, 2017

Uh oh!

saimn commented Nov 17, 2017

Uh oh!

taldcroft commented Nov 18, 2017

Uh oh!

saimn commented Nov 20, 2017

Uh oh!

taldcroft commented Nov 21, 2017

Uh oh!

saimn commented Nov 27, 2017

Uh oh!

taldcroft commented Nov 27, 2017

Uh oh!

saimn commented Nov 27, 2017

Uh oh!

pllim commented Nov 27, 2017

Uh oh!

taldcroft commented Nov 28, 2017

Uh oh!

saimn commented Nov 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

saimn commented Nov 28, 2017

Uh oh!

taldcroft left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

astropy-bot Bot commented Nov 15, 2017 •

edited

Loading

saimn commented Nov 15, 2017 •

edited

Loading

saimn commented Nov 28, 2017 •

edited

Loading

pllim commented Jan 26, 2018 •

edited

Loading