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

[REVIEW] Python and Cython style cleanup, pre-commit hook #41

Merged
merged 3 commits into from
Sep 9, 2019
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
8 changes: 4 additions & 4 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ DartConfiguration.tcl

## Python build directories & artifacts
dist/
cudf.egg-info/
cuspatial.egg-info/
python/build
python/*/build
python/cuspatial/*/bindings/**/*.cpp
python/cuspatial/*/bindings/**/*.h
python/cuspatial/*/bindings/.nfs*
python/cuspatial/*/_lib/**/*.cpp
python/cuspatial/*/_lib/**/*.h
python/cuspatial/*/_lib/.nfs*
python/cuspatial/*.ipynb
python/cuspatial/.ipynb_checkpoints
.Python
Expand Down
23 changes: 23 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
repos:
- repo: https://github.com/timothycrosley/isort
rev: 4.3.21
hooks:
- id: isort
- repo: https://github.com/ambv/black
rev: stable
hooks:
- id: black
- repo: https://gitlab.com/pycqa/flake8
rev: 3.7.7
hooks:
- id: flake8
- repo: https://gitlab.com/pycqa/flake8
rev: 3.7.7
hooks:
- id: flake8
alias: flake8-cython
name: flake8-cython
args: ["--config=python/cuspatial/.flake8.cython"]
types: [cython]
default_language_version:
python: python3
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

- PR #31 Add Github CODEOWNERS
- PR #39 Add cython headers to install, python / cmake packaging cleanup
- PR #41 Python and Cython style cleanup, pre-commit hook

## Bug Fixes

Expand Down
29 changes: 29 additions & 0 deletions ci/checks/style.sh
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,32 @@ else
echo -e "\n\n>>>> PASSED: isort style check\n\n"
fi

if [ "$BLACK_RETVAL" != "0" ]; then
echo -e "\n\n>>>> FAILED: black style check; begin output\n\n"
echo -e "$BLACK"
echo -e "\n\n>>>> FAILED: black style check; end output\n\n"
else
echo -e "\n\n>>>> PASSED: black style check\n\n"
fi

if [ "$FLAKE_RETVAL" != "0" ]; then
echo -e "\n\n>>>> FAILED: flake8 style check; begin output\n\n"
echo -e "$FLAKE"
echo -e "\n\n>>>> FAILED: flake8 style check; end output\n\n"
else
echo -e "\n\n>>>> PASSED: flake8 style check\n\n"
fi

if [ "$FLAKE_CYTHON_RETVAL" != "0" ]; then
echo -e "\n\n>>>> FAILED: flake8-cython style check; begin output\n\n"
echo -e "$FLAKE_CYTHON"
echo -e "\n\n>>>> FAILED: flake8-cython style check; end output\n\n"
else
echo -e "\n\n>>>> PASSED: flake8-cython style check\n\n"
fi

RETVALS=($ISORT_RETVAL $BLACK_RETVAL $FLAKE_RETVAL $FLAKE_CYTHON_RETVAL)
IFS=$'\n'
RETVAL=`echo "${RETVALS[*]}" | sort -nr | head -n1`

exit $RETVAL
16 changes: 8 additions & 8 deletions python/cuspatial/cuspatial/__init__.py
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
from .core.trajectory import (
derive,
distance_and_speed,
spatial_bounds,
subset_trajectory_id
)
from .core.gis import (
directed_hausdorff_distance,
haversine_distance,
lonlat_to_xy_km_coordinates,
point_in_polygon_bitmap,
window_points,
)
from .core.trajectory import (
derive,
distance_and_speed,
spatial_bounds,
subset_trajectory_id,
)
from .io.soa import (
read_uint,
read_its_timestamps,
read_points_lonlat,
read_points_xy_km,
read_polygon
read_polygon,
read_uint,
)
30 changes: 23 additions & 7 deletions python/cuspatial/cuspatial/_lib/soa_readers.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,26 @@ from cudf._lib.cudf cimport *
from libcpp.pair cimport pair

cdef extern from "soa_readers.hpp" namespace "cuspatial" nogil:
cdef gdf_column read_uint32_soa(const char *pnt_fn) except +
cdef gdf_column read_timestamp_soa(const char *ts_fn) except +
cdef pair[gdf_column, gdf_column] read_lonlat_points_soa(const char *pnt_fn) except +
cdef pair[gdf_column, gdf_column] read_xy_points_soa(const char *pnt_fn) except +
cdef void read_polygon_soa(const char *ply_fn, gdf_column* ply_fpos,
gdf_column* ply_rpos, gdf_column* ply_x,
gdf_column* ply_y) except +
cdef gdf_column read_uint32_soa(
const char *pnt_fn
) except +

cdef gdf_column read_timestamp_soa(
const char *ts_fn
) except +

cdef pair[gdf_column, gdf_column] read_lonlat_points_soa(
const char *pnt_fn
) except +

cdef pair[gdf_column, gdf_column] read_xy_points_soa(
const char *pnt_fn
) except +

cdef void read_polygon_soa(
const char *ply_fn,
gdf_column* ply_fpos,
gdf_column* ply_rpos,
gdf_column* ply_x,
gdf_column* ply_y
) except +
82 changes: 54 additions & 28 deletions python/cuspatial/cuspatial/_lib/soa_readers.pyx
Original file line number Diff line number Diff line change
@@ -1,78 +1,104 @@
# Copyright (c) 2019, NVIDIA CORPORATION.

# cython: profile=False
# distutils: language = c++
# cython: embedsignature = True
# cython: language_level = 3


from cudf.core.column import Column
from cudf._lib.cudf import *
from libc.stdlib cimport calloc, malloc, free
from libcpp.pair cimport pair

cpdef cpp_read_uint_soa(soa_file_name):
# print("in cpp_read_id_soa, reading ",soa_file_name)
cdef bytes py_bytes = soa_file_name.encode()
cdef char* c_string = py_bytes
cdef gdf_column c_id

with nogil:
c_id = read_uint32_soa(c_string)
c_id = read_uint32_soa(
c_string
)

id_data, id_mask = gdf_column_to_column_mem(&c_id)
id=Column.from_mem_views(id_data,id_mask)
id = Column.from_mem_views(id_data, id_mask)

return id

cpdef cpp_read_ts_soa(soa_file_name):
# print("in cpp_read_ts_soa, reading ",soa_file_name)
cdef bytes py_bytes = soa_file_name.encode()
cdef char* c_string = py_bytes
cdef gdf_column c_ts

with nogil:
c_ts = read_timestamp_soa(c_string)
c_ts = read_timestamp_soa(
c_string
)

ts_data, ts_mask = gdf_column_to_column_mem(&c_ts)
ts=Column.from_mem_views(ts_data,ts_mask)
ts = Column.from_mem_views(ts_data, ts_mask)

return ts

cpdef cpp_read_pnt_lonlat_soa(soa_file_name):
# print("in cpp_read_pnt_lonlat_soa, reading ",soa_file_name)
cdef bytes py_bytes = soa_file_name.encode()
cdef char* c_string = py_bytes
cdef pair[gdf_column, gdf_column] columns

with nogil:
columns = read_lonlat_points_soa(c_string)
columns = read_lonlat_points_soa(
c_string
)

lon_data, lon_mask = gdf_column_to_column_mem(&columns.first)
lon=Column.from_mem_views(lon_data,lon_mask)
lon = Column.from_mem_views(lon_data, lon_mask)
lat_data, lat_mask = gdf_column_to_column_mem(&columns.second)
lat=Column.from_mem_views(lat_data,lat_mask)
lat = Column.from_mem_views(lat_data, lat_mask)

return lon,lat
return lon, lat

cpdef cpp_read_pnt_xy_soa(soa_file_name):
# print("in cpp_read_pnt_xy_soa, reading ",soa_file_name)
cdef bytes py_bytes = soa_file_name.encode()
cdef char* c_string = py_bytes
cdef pair[gdf_column, gdf_column] columns

with nogil:
columns = read_xy_points_soa(c_string)
columns = read_xy_points_soa(
c_string
)

x_data, x_mask = gdf_column_to_column_mem(&columns.first)
x=Column.from_mem_views(x_data,x_mask)
x = Column.from_mem_views(x_data, x_mask)
y_data, y_mask = gdf_column_to_column_mem(&columns.second)
y=Column.from_mem_views(y_data,y_mask)
y = Column.from_mem_views(y_data, y_mask)

return x,y
return x, y

cpdef cpp_read_polygon_soa(soa_file_name):
# print("in cpp_read_polygon_soa, reading ",soa_file_name)
cdef bytes py_bytes = soa_file_name.encode()
cdef char* c_string = py_bytes
cdef gdf_column* c_ply_fpos=<gdf_column*>malloc(sizeof(gdf_column))
cdef gdf_column* c_ply_rpos=<gdf_column*>malloc(sizeof(gdf_column))
cdef gdf_column* c_ply_x=<gdf_column*>malloc(sizeof(gdf_column))
cdef gdf_column* c_ply_y=<gdf_column*>malloc(sizeof(gdf_column))
cdef gdf_column* c_ply_fpos = <gdf_column*>malloc(sizeof(gdf_column))
cdef gdf_column* c_ply_rpos = <gdf_column*>malloc(sizeof(gdf_column))
cdef gdf_column* c_ply_x = <gdf_column*>malloc(sizeof(gdf_column))
cdef gdf_column* c_ply_y = <gdf_column*>malloc(sizeof(gdf_column))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're leaking these 4, will file an issue and submit a follow up PR.


with nogil:
read_polygon_soa(c_string,c_ply_fpos, c_ply_rpos, c_ply_x, c_ply_y)
read_polygon_soa(
c_string,
c_ply_fpos,
c_ply_rpos,
c_ply_x,
c_ply_y
)

f_data, f_mask = gdf_column_to_column_mem(c_ply_fpos)
f_pos=Column.from_mem_views(f_data,f_mask)
f_pos = Column.from_mem_views(f_data, f_mask)
r_data, r_mask = gdf_column_to_column_mem(c_ply_rpos)
r_pos=Column.from_mem_views(r_data,r_mask)
r_pos = Column.from_mem_views(r_data, r_mask)
x_data, x_mask = gdf_column_to_column_mem(c_ply_x)
x=Column.from_mem_views(x_data,x_mask)
x = Column.from_mem_views(x_data, x_mask)
y_data, y_mask = gdf_column_to_column_mem(c_ply_y)
y=Column.from_mem_views(y_data,y_mask)

return f_pos,r_pos,x,y
y = Column.from_mem_views(y_data, y_mask)

return f_pos, r_pos, x, y
51 changes: 31 additions & 20 deletions python/cuspatial/cuspatial/_lib/spatial.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -9,33 +9,44 @@ from cudf._lib.cudf cimport *
from libcpp.pair cimport pair

cdef extern from "point_in_polygon.hpp" namespace "cuspatial" nogil:
cdef gdf_column point_in_polygon_bitmap(const gdf_column& pnt_x,
const gdf_column& pnt_y,
const gdf_column& ply_fpos,
const gdf_column& ply_rpos,
const gdf_column& ply_x,
const gdf_column& ply_y) except +
cdef gdf_column point_in_polygon_bitmap(
const gdf_column& pnt_x,
const gdf_column& pnt_y,
const gdf_column& ply_fpos,
const gdf_column& ply_rpos,
const gdf_column& ply_x,
const gdf_column& ply_y
) except +

cdef extern from "coordinate_transform.hpp" namespace "cuspatial" nogil:
cdef pair[gdf_column, gdf_column] lonlat_to_coord(const gdf_scalar cam_x,
const gdf_scalar cam_y,
const gdf_column & in_x,
const gdf_column & in_y) except +
cdef extern from "coordinate_transform.hpp" namespace "cuspatial" nogil:
cdef pair[gdf_column, gdf_column] lonlat_to_coord(
const gdf_scalar& cam_x,
const gdf_scalar& cam_y,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These weren't pass by reference before, but the .hpp file had them as pass by reference so I updated it. If incorrect please let me know.

const gdf_column& in_x,
const gdf_column& in_y
) except +

cdef extern from "haversine.hpp" namespace "cuspatial" nogil:
gdf_column haversine_distance(const gdf_column& x1, const gdf_column& y1,
const gdf_column& x2, const gdf_column& y2) except +
cdef extern from "haversine.hpp" namespace "cuspatial" nogil:
gdf_column haversine_distance(
const gdf_column& x1,
const gdf_column& y1,
const gdf_column& x2,
const gdf_column& y2
) except +

cdef extern from "hausdorff.hpp" namespace "cuspatial" nogil:
gdf_column& directed_hausdorff_distance(const gdf_column& coor_x,
const gdf_column& coor_y,
const gdf_column& cnt)except +
cdef extern from "hausdorff.hpp" namespace "cuspatial" nogil:
gdf_column& directed_hausdorff_distance(
const gdf_column& coor_x,
const gdf_column& coor_y,
const gdf_column& cnt
) except +

cdef extern from "query.hpp" namespace "cuspatial" nogil:
cdef pair[gdf_column, gdf_column] spatial_window_points(
cdef pair[gdf_column, gdf_column] spatial_window_points(
const gdf_scalar& left,
const gdf_scalar& bottom,
const gdf_scalar& right,
const gdf_scalar& top,
const gdf_column& x,
const gdf_column& y) except +
const gdf_column& y
) except +
Loading