Skip to content

Split single ufuncs.c file in multiple files#33

Merged
caspervdw merged 2 commits intopygeos:masterfrom
jorisvandenbossche:split
Aug 30, 2019
Merged

Split single ufuncs.c file in multiple files#33
caspervdw merged 2 commits intopygeos:masterfrom
jorisvandenbossche:split

Conversation

@jorisvandenbossche
Copy link
Copy Markdown
Member

This is an attempt to split the single (big) ufuncs C file in multiple files.

I find this a bit easier to understand the code if it is not all in a huge file, but it was also just a good exercise to get familiar with the code, so @caspervdw you certainly don't need to like this ;)

@caspervdw
Copy link
Copy Markdown
Member

This is great, thanks for doing this work. I am surprised AppVeyor isn't picking up this PR, probably I switched PR building off for some reason.

src/pygeom.c Outdated
PyErr_Format(PyExc_TypeError, "Expected string or bytes, found %s", value->ob_type->tp_name);
return NULL;
}
ptr = GEOSGeom_clone_r(context_handle, arg);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This code is unreachable! Should be deleted. My bad, I think

@caspervdw
Copy link
Copy Markdown
Member

I now realize that all feedback is related to stuff I wrote myself. Feel free to merge as is (except for the compiler warning, would be nice to solve that)

@jorisvandenbossche
Copy link
Copy Markdown
Member Author

No problem, I can address those at once when fixing the other issue.

One additional thought I had is if we want to keep the "ufuncs" name for the compiled module in Python? Since it already contains more as just the ufuncs (it also already contains the base Geometry type), and might contain more in the future (dtype, STRTree, other algos like a join).
Eg, we could have a single compiled module pygeos.lib.

@jorisvandenbossche
Copy link
Copy Markdown
Member Author

Or, we could actually also expose them as multiple python modules, although I don't know how that works with interlinking between them on the C level if they are not compiled into a single linked module.

Anyway, in the end, users don't directly use this but rather the wrapped python functions, so the name is also not that important.

@caspervdw caspervdw merged commit 5202ade into pygeos:master Aug 30, 2019
@caspervdw
Copy link
Copy Markdown
Member

One additional thought I had is if we want to keep the "ufuncs" name for the compiled module in Python? Since it already contains more as just the ufuncs (it also already contains the base Geometry type), and might contain more in the future (dtype, STRTree, other algos like a join).
Eg, we could have a single compiled module pygeos.lib.

I like the idea of making this more modular on the C level, and expose it with pygeos.lib as a single module. Something for a next round of refactor.

jorisvandenbossche pushed a commit to jorisvandenbossche/shapely that referenced this pull request Nov 29, 2021
Split single ufuncs.c file in multiple files
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.

2 participants