Skip to content

Basic code coverage#63

Merged
ajfriend merged 7 commits intouber:cythonfrom
ajfriend:cython
Sep 25, 2019
Merged

Basic code coverage#63
ajfriend merged 7 commits intouber:cythonfrom
ajfriend:cython

Conversation

@ajfriend
Copy link
Collaborator

Currently only covers .py files. Ideally, we'd get this to also compute coverage on Cython files.

@codecov
Copy link

codecov bot commented Sep 25, 2019

Codecov Report

❗ No coverage uploaded for pull request base (cython@2397ebd). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             cython      #63   +/-   ##
=========================================
  Coverage          ?   80.29%           
=========================================
  Files             ?        6           
  Lines             ?      137           
  Branches          ?        0           
=========================================
  Hits              ?      110           
  Misses            ?       27           
  Partials          ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2397ebd...87b69b2. Read the comment docs.


from cpython cimport bool
from libc.stdint cimport int64_t
from h3lib cimport bool, int64_t, H3int
Copy link
Contributor

Choose a reason for hiding this comment

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

Should bool and int64_t still be imported from cpython and libc.stdint?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These types seemed out of place in h3core.pyx, so I wanted to put all the type declarations in one place. h3lib.pxd seemed like the best candidate.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it should be an issue to import types in pyx files, with this it feels like we're doing stdint.pxd->h3lib.pxd->h3core.pyx instead of stdint.pxd->h3core.pyx

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's exactly what we're doing ;)

I'm thinking of it more like (whatever random sources) -> h3lib.pxd -> h3core.pyx. All types are then defined in h3lib.pxd.

I like that better than having types come from three different sources (cpython, stdint, h3lib) -> h3core.pyx.

If other .pyx files need to use the same types, they can all import from h3lib, without having to remember they're hidden away in cpython or stdint.

Copy link
Contributor

@ankitmehta21 ankitmehta21 left a comment

Choose a reason for hiding this comment

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

Looks good, just one comment re: type imports.
Have you tried .coveragerc for cython code coverage:
http://blog.behnel.de/posts/coverage-analysis-for-cython-modules.html

@ajfriend
Copy link
Collaborator Author

I took a stab at Cython code coverage, but couldn't quite figure out how to get the build working correctly.

I was also looking at this page: https://medium.com/@dfdeshom/better-test-coverage-workflow-for-cython-modules-631615eb197a

I'll put up Cython code coverage as an issue, and we'll see who gets to it first :)

@ajfriend ajfriend merged commit fdacf99 into uber:cython Sep 25, 2019
ajfriend added a commit that referenced this pull request May 10, 2020
* Use Hunter

* Use position independent code to link shared object correctly

* Fix issues in module naming

* Temporary fix

* Use src directory and fix CMake build for subdirectory

* Fix build, remove dependency on Hunter

* aj first stab at cython

* Added class to manage H3Index array memory.
Added a bunch of wrapping functions.

* hacky version of polyfill working

* remove some naming redundancy

* Adding compact and uncompact functions

* notes

* clean up HexMem class

* reworked geofence and geopolygon. now memorysafe (i think)

* reorder definitions

* formatting

* Use scikit-build for setup

* Changes for getting build partially working on macOS

* Add scikit-build to requirements-dev.txt

* Build the core library as static and update notes.md

* update cython notes

* Cleaning up the Cython instructions and tests a bit

* cleaning out extra files for now

* readme

* Cleaning out more files to focus on Cython dev in this branch

* makefile

* simplify install process with makefile

* adding back in a bunch of files

* Add appveyor.yml for Cython

* Disable Python 2.7 test

* reorganize cython build; working on os x

* get new changes running on my OSX machine

* add hexmem.pyx

* just throwing it all in h3core...

* newlines

* separate HexMem into separate module

* change import to `h3py.h3core` for tests

* update readme for new folder structure

* starting to layout separate APIs

* `set_int` and `set_str` working

* notes

* cleaning up a bit

* moving the HexMem helper functions

* moving lat/lng functions to separate module

* move output formatting functions

* use `size_t` in HexMem

* trying input/output formatters

* rename to make api modules obvious

* adding some functions providing hexagon stats

* adding some TODOs to use C functions instead of python

* fix api typos

* adds unidirectional edge functions

* cleaning up geo functions a bit

* add todo

* create `api` directory and move modules

* try using `_api_template.py` to generate different APIs

* remove HexMem class and use memoryviews of cython.view.array objects instead

* make single mercator function

* remove redundant exception handling

* automatic input validation

* add validation for unidirectional edges

* simplify API generation

* a little cleaning

* defaults for parent and child functions

* add back in H3str as input type to hex2int

* simplify validation and conversion code

* adding tests for geo functions

* rename hexmem -> util

* move exceptions to util.pyx

* move geo functions to a separate module

* move hex2int and int2hex to each api

* rename validation functions

* rename h3api -> libh3 to reserve "api" for the different python APIs we expose

* cleaning up memory allocation

* fix clean up code

* clean up comments

* add some resolution checking

* adding a diagram of the code to remove zeros

* adding some docstrings

* update H3 C library

* add line function

* docstring

* some function name changes

* rename k_ring -> disk

* rename hex_ring -> ring

* cleaning up modules

* ... cleaning up all the modules

* add in backward-compatible `old` API

* add some thoughts on slower fallback for `ring`

* some naming changes

* fix up mean_hex_area and mean_edge_length

* rename base_cell -> get_base_cell

* remove special case for n=0 memory allocation

* rename edge_hexes -> edge_cells

* rename edges_from_hex -> edges_from_cell

* rename functions to match current python implementation

* remove h3py.api.old in favor of defaulting to set_str

* update resolution docstring

* use macro in CMake (I have no idea what I'm doing tho...)

* try this one weird trick for Python 2 compatibility!

* add note on .rstrip('L') for handling python 2

* Use `__version__.py` (#57)

* use __version__.py to define python version

* add Python/C version check test

* newline

* some questions on the CMake file

* actually, use _version.py for less confusing naming

* note

* Change directory structure, make tests work (#61)

* travis with multiple python versions (#62)

* Add appveyor.yml for Cython

* Disable Python 2.7 test

* Use `__version__.py` (#57)

* use __version__.py to define python version

* add Python/C version check test

* newline

* some questions on the CMake file

* actually, use _version.py for less confusing naming

* note

* Change directory structure, make tests work (#61)

* linting

* linting

* fix purge command

* clean up requirements a bit

* messing with travis

* more travis

* try travis again

* clean up requirements a bit more

* test more python versions, including 2.7

* cmake version thing

* py3.5 doesn't seem to work with cmake

* run all tests in test directory

* remove fabfile

* make sure fails on lint

* get lint to pass

* fix purge command

* bare bones requirements-dev.txt

* use `pytest tests/*` in CI configs

* no need for a `MANIFEST.in` anymore

* fix windows wildcard?

* whatever you want, Windows.

* Basic code coverage (#63)

* basic code coverage

* rename error types and drop prefix on h3ilb.H3int

* Rework some internal names (#65)

* rename `h3utils` to `util`

* typo in cmakelists

* rename `h3_core` to `memview_int` to match format of other interfaces

* linting passes on tests

* make sure travis lints the tests

* getting better coverage on the interfaces

* newline

* add `array_int` (numpy) tests

* install numpy

* make sure appveyor is running all the tests

* dev readme changes

* setup.py changes

* more setup.py changes

* Polyfill working on polygons with holes (#71)

* jupyter lab makefile target for testing

* got polyfill with holes working

* linting

* docstring for h3_to_geo

* improve coverage

* ignoring more flake rules, whilst listening to one

* make polyfill resolution a required positional argument

* use boolean lnglat_order instead of string

* move polyfill tests to own file

* test polyfill polygon input permutations (clockwise vs counterclockwise elements and closed or open loops)

* update description of polyfill inputs

* comment on test

* lint things up

* add some test comments

* don't need to check for null before free

* Add `h3_set_to_multi_polygon` (#74)

* got to_multipolygon workin

* clean up code and add a simple test

* don't need this import

* moved the to_multipoly stuff to own file

* got the geo_json bit working

* add some tests

* linting

* file rename

* update renaming thoughts

* clean up usage of H3int

* no globs!

* don't need this "mercator" business

* rename `check_addr` to `check_cell`

* `destroyLinkedPolygon` note

* rename geo2coord to deg2coord and coord2geo to coord2deg

* Get original test suite working (#79)

* got to_multipolygon workin

* clean up code and add a simple test

* don't need this import

* moved the to_multipoly stuff to own file

* got the geo_json bit working

* add some tests

* linting

* file rename

* update renaming thoughts

* clean up usage of H3int

* getting some more of the original tests to work

* get a bunch of tests working!

* got some more tests working

* got all but one test passing; modified some tests that no longer need to fail

* update test, based on what I think the right result is

* update test for uni edge

* all linty fresh now

* split `out_collection` into `out_ordered` and `out_unordered`

* upgrade to h3core 3.6.1

* bump version

* add notes on empty array inputs

* 100% code coverage (#82)

* add int/str conversion test

* 100% codecov!

* linting

* remove notes

* to `not` or not to `not`

* gotta have a badge!

* Get tox working (#83)

* use relative imports

* clean up some messages

* simplify dynamic api modules a bit

* making some adjustments for tox

* update appveyor script

* remove linux wheel-building code, as i think it is out of date

* fix edge_is_valid test

* linting

* add some badges

* mess with travisci a little bit

* more messing with travis

* flake8 issue

* continue trying to fix flake issue

* we'll get it eventually

* messing with cmake

* revert the travis.yml file; add flake8 to tox

* and revert cmake pin

* messing with appveyor

* revert appveyor change. not sure how to wildcard in appveyor

* separate flake8 in tox envs

* move `cell` and `edge` functions to separate files

* have api import almost everything from core.py

* get outta here, flake8!

* clean up `_api_template.py`

* clean up int -> hex conversion function

* notes on `move_nonzeros()`

* more notes

* refactor so that all cython code is used via `h3.core`

* move noqa

* Convert original test suite to use pytest syntax (#84)

* convert original test suite to use pytest syntax

* making some key hexes obvious

* have travis print the full stack trace on error

* make a few more hex keys obvious

* rework test_compact_malformed_input a bit

* lint!

* formatting

* adding back in a few comments

* move uses of `map` to list comprehensions

* Slightly improve invalid index error reporting (#86)

* slight improvement to invalid index error reporting

* linting

* install notes

* parallel tox doesn't seem that much faster

* use relative imports

* hide h3.core from user api namespace

* noqa on __init__.py

* hides internal modules, but seems to break tox

* remove the testing notebook

* revert files back

* rename `core` to `internal_api`

* add the H3 error types to the public API (for testing)

* add a few function docstrings

* rework errors in tests

* address some comments

* have `create_ptr` do an `except? NULL`, which should be faster than `*`

* version docstring

* Get _cy folder layout working (#87)

* try to get _cy folder layout working

* Fix CMake build

* specify the LIBRARY DESTINATION to be the _cy folder

* move `_internal_api` to `_cy/__init__.py`

* remove version checks in CMakeLists.txt (#88)

* Better API names (#89)

* rename APIs

* rename test files

* remove obsolete comments

* update test files in appveyor

* Use `basic_` prefix instead of `py_`

* makefile

* appveyor!

* API docs

* linting

* tweak docstrings

* missed one!

* Immutability questions for the future

* linting

* module docstrings have to go first

* clarifying API collection input expectations

* input collection tests

* Add more docstrings to the APIs (#91)

* new docstrings

* use pandas docstring guide formatting for the first few functions

* Adding a few more docstrings

* addressing comments

* taking care of comments

* use `H3Cell` instead of `H3Index` to be more precise

* Update h3_get_base_cell

* `h3_is_res_class_III` docstring

* spacing

* move todos to comments

* notes on collections

* module docstring

* polyfill and to_multi_polygon docs

* clean up some docstrings

* address comments

* Segfaults (#92)

* making some segfault notes on `polyfill`

* Allow Cython to handle the error

* notes on tests

* rename test to `test_test_invalid_polygon` and group into single test

* typo

* Add `get_pentagons` function (#95)

* add `get_pentagons`

* linting

* rename to `get_pentagon_indexes`

* Prep cython branch readme before merging cython into master (#98)

* needs more badges!

* prep the readme before merge to master

* add logo

* remove docs folder

* use hex notation in the integer interface test

* conforming to the tyranny of `E222 multiple spaces after operator`

* adding long description to setup.py

* lint

* python 2.7 compatibility

* cell/edge boundary: clean up geo_json branch

* describe why we use `hex` in the error messages instead of `int2hex`

* geofence free notes

* update changelog

* typo

* alg -> algorithm

* remove irrelevant todo

* add distance >= 0 error checking

* add a missing file to appveyor tests

* trying out appveyor

* why won't you listen to reason, appveyor!?

Co-authored-by: Isaac Hier <ihier@uber.com>
Co-authored-by: Isaac Hier <isaachier@gmail.com>
Co-authored-by: Isaac Brodsky <isaac@uber.com>
Co-authored-by: Isaac Brodsky <isaac@isaacbrodsky.com>
Co-authored-by: Kurt Smith <kusmith@homeaway.com>
Co-authored-by: ankitmehta21 <ankitmehta@uber.com>
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.

3 participants