Skip to content
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

fix: crash on null values from access long type #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

verasativa
Copy link

Changed the conversion from access long type to pandas float instead of pandas int, in order to support null values.

Note that pandas just released int null support @ pandas .24, but still experimental.

Changed the conversion from access long type to pandas float instead of pandas int, in order to support null values.

Note that pandas just released int null support @ pandas .24, but still experimental.
verasativa added a commit to verasativa/defunciones-decoder that referenced this pull request Feb 25, 2019
@@ -34,7 +34,8 @@ def _extract_dtype(data_type):
if data_type.startswith('double'):
return np.float_
elif data_type.startswith('long'):
return np.int_
# access CAN have null values on long type, @ pandas 0.24 int null suport is experimental, a float is safer for now.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

float supports a smaller range than INT. This might be ok, but when dealing with IDs (which tend to use up the whole range of values), you will break code.

consider:

f = np.float_(1 << 53)
print(f)
f += 1
print(f)

(in case np.float_ is a 64 bit IEEE representation)

So this should at least be optional.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since pandas .24 there is a Int type (notice the caps on the I), which allows nulls ints. If you are willing to require .24+ (they are at .24.2 now), that will fix it for both cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants