Skip to content

Duplicate xstrtod implementations #19361

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

Closed
jbrockmendel opened this issue Jan 23, 2018 · 8 comments · Fixed by #25925
Closed

Duplicate xstrtod implementations #19361

jbrockmendel opened this issue Jan 23, 2018 · 8 comments · Fixed by #25925
Labels
Clean Internals Related to non-user accessible pandas implementation
Milestone

Comments

@jbrockmendel
Copy link
Member

In _libs/src/parse_helper.h and libs/src/parser/tokenizer.c

Not identical, comments suggest the tokenizer.c version may be more modern.

@chris-b1
Copy link
Contributor

The core functionally is the same, differences are the tokenizer.c version handles a thousands separator, the parse_helper.c version has a maybe_int out variable, agree they could be combined.

@chris-b1 chris-b1 added Internals Related to non-user accessible pandas implementation Difficulty Intermediate Clean labels Jan 23, 2018
@chris-b1 chris-b1 added this to the Next Major Release milestone Jan 23, 2018
@jbrockmendel
Copy link
Member Author

@chris-b1 There are .gitignore and Makefile files in _libs/src/parser. Are those used for something or remnants from the Old Days?

@chris-b1
Copy link
Contributor

Yeah, definitely something very old, I think those could be cleaned out too.

@jbrockmendel
Copy link
Member Author

It also looks like src/klib/ktypes.h and kvec.h are unused, though this should be checked by someone who understands C namespaces.

@chris-b1
Copy link
Contributor

Yeah, those are unused as well - for good and bad C doesn't have namespaces, (or modules, or imports, etc) - all it can really do is textual inclusion, so if those aren't #include'ed somewhere, they aren't used.

@jbrockmendel
Copy link
Member Author

_libs.groupby.kth_smallest_c almost identical to _libs.algos.kth_smallest

@jbrockmendel
Copy link
Member Author

@chris-b1 hopefully a softball question: Currently util gets its INT32_MAX etc constants from headers/stdint.h. headers/stdint.h contains special-casing for windows systems. Is that special casing relevant for UINT64_MAX etc, or can we use the (much more convenient for current purposes) from libc.stdint cimport ...?

@chris-b1
Copy link
Contributor

chris-b1 commented Feb 2, 2018

Yes, that special casing is required - cython definitions look for a 'stdint.h' which the Windows toolchain for 2.7 doesn't have, so the build would fail.
https://www.microsoft.com/en-us/download/details.aspx?id=44266

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Internals Related to non-user accessible pandas implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants