amras1/astropy — pull request #2 by taldcroft (2014-03-21 21:49:34+00:00)
Title: Suggested updates for HTML reader code

@amras1 - I spent some time looking through the HTML reader code in detail (haven't looked at the writer yet). Overall it looks great.

In going through this I noticed a couple of things that were a little subtle and I couldn't tell exactly what to do without trying it myself. Also my testing generated some new test cases, so in the end I wrote some code and I'm now issuing a pull request on *your* branch. You can look over the changes (probably one commit at a time to understand best) and then merge into your branch. This will then get reflected in your main astropy PR.

The second commit (f85caa5) is the most substantive. In your version you were doing the type conversion (from strings to the appropriate numpy type) before putting the values into data structures for subsequent processing by `TableOutputter`. What can happen is that if any of the values in a multi-span column is a string, then all of them need to be treated as strings. But if one of the original table elements was `2.5000000000001` then it can get converted into a float and then ends up in the final table as a string but `2.5`. This is really a corner case, but the main point is that in the `HTMLOutputter` it is important to leave everything as strings and let `TableOutputter` do the numpy type conversion.

In 6fd4026 I changed from `soup.string` to `soup.text` since the latter always gives a string (possibly zero length). This let's the normal missing value machinery work.

Finally in db149cc I added a test that defines how I would propose to handle the case when the specified `table_id` is not found. Currently this case will fail out with a message: `No start line found for HTML data`. This won't help users understand exactly what the problem is, so the new test defines what they should get in this case.