Change default ASCII reader numeric types to 64 bits on all platforms #4686
Conversation
| from collections import defaultdict, OrderedDict | ||
| from textwrap import wrap | ||
| from warnings import warn | ||
| import numpy as np |
There was a problem hiding this comment.
Ooops, this is a leftover, will remove.
|
This should have a Changes entry since it's an API change for some users. |
|
I'm sure there is some reason that 32 bit is the default on windows - I assume there is a performance penalty to using 64 bit data types on a 32 bit system. There are few 32 bit systems left out there, so that's probably not an issue any longer; thus, I'm fine with this change (once the tests pass). |
|
Drat, it's failing in the fast reader. @mdmueller - do you know offhand how to change the default int output to be int64 instead of int32 on windows in the fast reader? |
|
@mdmueller @hamogu - Just pushed up a fix that might work. |
taldcroft
force-pushed
the
ascii-int64-float64
branch
from
1f9c4eb to
d8ecc13
Compare
|
Hi humans 👋 - this pull request hasn't had any new commits for approximately 1 year, 6 months. I plan to close this in a month if the pull request doesn't have any new commits by then. In lieu of a stalled pull request, please close this and open an issue instead to revisit in the future. Maintainers may also choose to add If you believe I commented on this issue incorrectly, please report this here. |
|
⏰ Time's up! ⏰ I'm going to close this pull request as per my previous message. If you think what is being added/fixed here is still important, please remember to open an issue to keep track of it. Thanks! If this is the first time I am commenting on this issue, or if you believe I closed this issue incorrectly, please report this here. |
3 participants
#4684 raised the issue of how the different underlying type for
np.inton Windows (np.int32) vs. other platforms (np.int64) can be a problem. To solve that particular issue, and more generally ensure consistent behavior for reading ASCII tables across platforms, this changes the default types to specifically choose the 64-bit version, e.g.np.int64instead ofnp.int.This is going to be an API change for Windows users. I think it should be acceptable, but comments otherwise are welcome.
Fixes #4684.