Repository navigation
Ignore parallel option in fast C reader - #10880
Conversation
271f56d to
e3736b6
Compare
| ^^^^^^^^^^^^^^^^ | ||
|
|
||
| - Partially fixed a performance issue when reading in parallel mode. Parallel | ||
| reading does not work correctly and has worse performance than serial |
There was a problem hiding this comment.
When you say "does not work correctly" that could imply it is returning the wrong answer. I did not see any statements this the answer is wrong, just that it is slow. If so, I would reword this here and elsewhere saying that "Parallel reading currently has substantially worse performance than the default serial reading, so we now ignore ...".
| # deprecation warning since we may still fix it in future. | ||
| if parallel: | ||
| warnings.warn('parallel reading does not currently work, ' | ||
| 'so falling back to serial reading', AstropyWarning) |
There was a problem hiding this comment.
I think it might help anybody that hits this to have a reference to issue #8858 in the warning text.
|
About the milestone for removing the code, since the 4.2 feature freeze is 7 days away that seems entirely unrealistic. I'd just start off with the milestone of 4.3 for either fixing or removing the parallel code. |
50f31ee to
1dd755c
Compare
|
@taldcroft - I think your comments are addressed now |
…an serial reading, so we now ignore the parallel option and fall back to serial reading.
1dd755c to
3518df8
Compare
Ignore parallel option in fast C reader
Ignore parallel option in fast C reader
Ignore parallel option in fast C reader
As discussed in #8858, this ignores for now the parallel option in the fast C reader in io.ascii, since serial reading has better performance. The warning is deliberately not a deprecation warning as this may be fixed in future.
This does mean that we now have some code that isn't tested and I think it's important that we simply remove this code if the bug isn't fixed in a timely way, so I'll open an issue to remind us to remove this code, and I'll milestone it for 4.2 initially though we could discuss what a good time frame is. I think it makes sense to be reasonably aggressive or we'll forget about it. In any case we should keep this present PR simple and we can backport it to LTS and 4.1.
Running the test suite before this PR on MacOS X with Python 3.8:
and with this PR:
Fixes #9864