Move C++ code to Cython #87
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
untested also dropped windows newlines
also all but one test is now passing
maybe even on travis
they are now defined as strings in c++ rather than having enums in c++ and strings in python which get redefined as ints. this means they're only defined once and you can call get_available_aggregations and get_available_decays on the cython wrapper to get them from the C++ definitions
Changed a couple of data structures to support std::string as keys for the containers of the POIs. It doesn't compile yet, there are some issues with the Cython files.
Changed the `operator[]` to `insert()` calls because the map value type doesn't have a default constructor.
Given the general style of the existing codebase, I migrated my use of exceptions to presence test of elements in containers. Still, it would be a good idea to define a behavior when the methods are called with a non-existent element. The first step for that, it would be to define a behavior for that in the python side.
In network.py, changing the call to send the string version of the category instead of an integer. In the test, also changing the test category to send a string instead of a number.
The `map::insert` call in `libch.cpp` was being called with an existing network, with POIs defined. The behavior of the method in that case is to do nothing, because the key is already defined. Now, the element is checked for presence and in case it exists it is removed and then replaced. I removed the creation of an array of POI data structures from the code. The initPOIs method is still present because it defines two values (max_distance and max_items). @fscottfoti We need to define if those values are actually useful, and in that case if we should pass when creating the network, or at other initialization step to avoid having to call this particular method.
Now the max_distance parameter (double) and the max_pois (integer) are defined per category in the `initialize_category` call.
There are some issues with the handling in Python3 of the `bytes` and `string` types. When that gets to the `cython` wrapper it generates and error. Now, there are some more explicit conversions in the code, and explicit literals when calling directly the functions on the tests.
By suggestion of @fscottfoti. Category parameter is now the first one, leaving to the second and third place for maxitems and maxdistance.
added notes on reserve_num_graphs
Remove the need of POI initialization call
This was referenced Jun 24, 2017
Closed
This was referenced Jun 24, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a major rewrite of the layer between python and C++, which was previously written using the numpy C++ API, and now is written in cython. The C++ that remains has been cleaned up a bit and formatted.
The major functionality change is that global memory is no longer used, so reserve_num_graphs no longer needs to be called and Network objects can be created and destroyed at the user's pleasure.