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

simplify skiplist inclusion/cimport to be more cythonize-friendly #18420

Merged
merged 6 commits into from
Nov 27, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions pandas/_libs/skiplist.pxd
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# -*- coding: utf-8 -*-
# cython: profile=False

from cython cimport Py_ssize_t

from numpy cimport double_t


cdef extern from "src/skiplist.h":
ctypedef struct node_t:
node_t **next
int *width
double value
int is_nil
int levels
int ref_count

ctypedef struct skiplist_t:
node_t *head
node_t **tmp_chain
int *tmp_steps
int size
int maxlevels

skiplist_t* skiplist_init(int) nogil
void skiplist_destroy(skiplist_t*) nogil
double skiplist_get(skiplist_t*, int, int*) nogil
int skiplist_insert(skiplist_t*, double) nogil
int skiplist_remove(skiplist_t*, double) nogil


# Note: Node is declared here so that IndexableSkiplist can be exposed;
# Node itself not intended to be exposed.
cdef class Node:
cdef public:
double_t value
list next
list width


cdef class IndexableSkiplist:
cdef:
Py_ssize_t size, maxlevels
Node head

cpdef get(self, Py_ssize_t i)
cpdef insert(self, double value)
cpdef remove(self, double value)
18 changes: 9 additions & 9 deletions pandas/_libs/src/skiplist.pyx → pandas/_libs/skiplist.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@

# Cython version: Wes McKinney

cdef extern from "math.h":
double log(double x)
from libc.math cimport log

# MSVC does not have log2!

Expand All @@ -16,6 +15,7 @@ cdef double Log2(double x):

cimport numpy as np
import numpy as np
from numpy cimport double_t

from random import random

Expand All @@ -25,10 +25,10 @@ np.import_array()
# TODO: optimize this, make less messy

cdef class Node:
cdef public:
double_t value
list next
list width
# cdef public:
# double_t value
Copy link
Contributor

Choose a reason for hiding this comment

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

why are you commenting these out? just to show what the structure is? I would remove this

Copy link
Contributor

Choose a reason for hiding this comment

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

aren't you supposed to declare the member variables in the .pyx, and not the .pxd?

Copy link
Member Author

Choose a reason for hiding this comment

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

aren't you supposed to declare the member variables in the .pyx, and not the .pxd?

AFAICT the rule is that if the class is declared in the .pxd, the member variables need to be declared there (and only there)

why are you commenting these out? just to show what the structure is? I would remove this

I leave these in place so we don't have to go back and check the pxd

# list next
# list width

def __init__(self, double_t value, list next, list width):
self.value = value
Expand All @@ -43,9 +43,9 @@ cdef class IndexableSkiplist:
Sorted collection supporting O(lg n) insertion, removal, and
lookup by rank.
"""
cdef:
Py_ssize_t size, maxlevels
Node head
# cdef:
# Py_ssize_t size, maxlevels
Copy link
Contributor

Choose a reason for hiding this comment

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

same

# Node head

def __init__(self, expected_size=100):
self.size = 0
Expand Down
22 changes: 0 additions & 22 deletions pandas/_libs/src/skiplist.pxd

This file was deleted.

17 changes: 6 additions & 11 deletions pandas/_libs/window.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,13 @@ cimport util

from libc.stdlib cimport malloc, free


from numpy cimport ndarray, double_t, int64_t, float64_t

from skiplist cimport (IndexableSkiplist,
node_t, skiplist_t,
skiplist_init, skiplist_destroy,
skiplist_get, skiplist_insert, skiplist_remove)

cdef np.float32_t MINfloat32 = np.NINF
cdef np.float64_t MINfloat64 = np.NINF

Expand All @@ -30,19 +34,10 @@ cdef inline int int_min(int a, int b): return a if a <= b else b

from util cimport numeric

from skiplist cimport (
skiplist_t,
skiplist_init,
skiplist_destroy,
skiplist_get,
skiplist_insert,
skiplist_remove)

cdef extern from "../src/headers/math.h":
double sqrt(double x) nogil
int signbit(double) nogil
double sqrt(double x) nogil

include "skiplist.pyx"

# Cython implementations of rolling sum, mean, variance, skewness,
# other statistical moment functions
Expand Down
8 changes: 5 additions & 3 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,7 @@ class CheckSDist(sdist_class):
'pandas/_libs/missing.pyx',
'pandas/_libs/testing.pyx',
'pandas/_libs/window.pyx',
'pandas/_libs/skiplist.pyx',
'pandas/_libs/sparse.pyx',
'pandas/_libs/parsers.pyx',
'pandas/_libs/tslibs/strptime.pyx',
Expand Down Expand Up @@ -544,6 +545,9 @@ def pxd(name):
'_libs.reshape': {
'pyxfile': '_libs/reshape',
'depends': _pxi_dep['reshape']},
'_libs.skiplist': {
'pyxfile': '_libs/skiplist',
'depends': ['pandas/_libs/src/skiplist.h']},
'_libs.sparse': {
'pyxfile': '_libs/sparse',
'depends': _pxi_dep['sparse']},
Expand Down Expand Up @@ -629,9 +633,7 @@ def pxd(name):
'pyxfile': '_libs/testing'},
'_libs.window': {
'pyxfile': '_libs/window',
'pxdfiles': ['_libs/src/skiplist', '_libs/src/util'],
'depends': ['pandas/_libs/src/skiplist.pyx',
'pandas/_libs/src/skiplist.h']},
'pxdfiles': ['_libs/skiplist', '_libs/src/util']},
'io.sas._sas': {
'pyxfile': 'io/sas/sas'}}

Expand Down