-
-
Notifications
You must be signed in to change notification settings - Fork 2k
Test io.ascii and table.Table write-read round-tripping #5235
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
Conversation
Use a hard-coded 'debug' variable to control level of output of tests in test_read_write.py
The writers and readers need slightly different kwargs. Use 'Writer' to seed 'Reader' or to define 'format'. Strip out kwargs that don't make sense to Reader or would confuse Reader (e.g., 'delimiter=None').
'create_reader_kwargs_from_writer_kwargs' creates a test_def with ['kwargs'] appropriate for Reading. by copying from a test_def, removing keys from hardcoded list of names, and handling the special case of 'delimiter=None'.
Each tested member of test_write.test_defs is now tracked as a separate test Each of 'io.ascii' and 'table.Table' is now a separate test.
This routine tests if a table can be written, and then read back. Thus the name test_write_read.py better reflects the order.
We can write HTML without BeautifulSoup but we can't read it back in.
|
@hamogu Here's the test round-trip write-read function. I would very much welcome your comments and feedback. |
|
@taldcroft I wrote this round-tripping test function for I appreciate your support of this concept and would welcome your feedback as well. |
| ] | ||
|
|
||
| test_def_masked_fill_value = [ | ||
| test_defs_masked_fill_value = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change? It sees to me that "test_def" is jsut as good as "test_defs" here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency with test_defs and test_defs_no_data. The prefix test_defs denotes a collection, while the individual test definition is the singular test_def.
"Pluralize test_defs_masked_fill_value for consistency"
|
I think this is a good think to test, specifically as we add features (e.g. the LaTeX writer used to round-trip in the past before we added support for units). Can we add fixes for the test failures to this PR though? It's not going to get merged with the tests passing. You can say "But it's already broken, the tests only confirm that." The problem is that then all tests for all other PRs would also show a "failed" status which makes it a lot harder to spot the really new failures. |
|
Lastly, can you squash the commits into one or two? (Let me know if you don't know how. I always for get, but I know where to look it up.) |
Adds a new test
test_write_read.pythat tests that alltest_defssamples intest_write.pycan be successfully written (to a StringIO object) and read back in to recover the correct data.Note that adding this test creates 3 new failures. This is a "success" of this feature branch. But will thus break master passing its tests.
The failures are
ascii.aastexdue to incorrect requirement for a '\\' on the last line of a table #5160 - "Table fails to round-tripascii.aastexdue to incorrect requirement for a '' on the last line of a table." There are two examples in the suite that trigger this.\begin{tabletype}[tablealign]example. I'm a bit more stuck on what will need to be fixed for this. I've never seen this LaTeX table type.The first two are the motivating reasons why I wrote this round-tripping test in the first place, so it's great that only other one thing came up.
[*] The log shows 12 failures, but these are each of the key 3 failures failing in each of
fast_writer=True, Falseand in each of theio.asciiandtable.Tableinterfaces.