home / github / issue_comments

Menu
  • Search all tables
  • GraphQL API

issue_comments: 1029296782

This data as json

html_url issue_url id node_id user created_at updated_at author_association body reactions issue performed_via_github_app
https://github.com/simonw/sqlite-utils/pull/385#issuecomment-1029296782 https://api.github.com/repos/simonw/sqlite-utils/issues/385 1029296782 IC_kwDOCGYnMM49WdKO 9599 2022-02-03T18:51:21Z 2022-02-03T18:51:21Z OWNER

What do you think about adding these as methods on the Database class instead? Then you could do:

```python

This is with an optional argument, which if omitted runs find_spatialite() for you:

db.init_spatialite()

Instead of:

init_spatialite(db, find_spatialite()) Likewise, the `add_geometry_column` and `create_spatial_index` methods could live on `Table`:python

Instead of this:

add_geometry_column(db["locations"], "POINT", "geometry") create_spatial_index(db["locations"], "geometry")

Could have this:

db["locations"].add_geometry_column("POINT") db["locations"].create_spatial_index("geometry") `` On the one hand, this is much more consistent with the existingsqlite-utils` Python API.

But on the other hand... this is mixing SpatiaLite functionality directly into the core classes. Is that a good idea, seeing as SpatiaLite is both an optional extension (which can be tricky to install) AND something that has a very different release cadence and quality-of-documentation from SQLite itself?

There's a third option: the SpatiaLite could exist on subclasses of Database and Table - so the above examples would look something like this:

```python from sqlite_utils.gis import SpatiaLiteDatabase

db = SpatiaLiteDatabase("geo.db") db.init_spatialite() db["locations"].add_geometry_column("POINT") db["locations"].create_spatial_index("geometry") ```

On the one hand, this would keep the SpatiaLite-specific stuff out of the core Database/Table classes. But it feels a bit untidy to me, especially since it raises the spectre of someone who was already subclassing Database for some reason now needing to instead subclass SpatiaLiteDatabase (not too keen on that capitalization) - or even (horror) trying to dabble with multiple inheritance, which can only lead to pain.

So I don't have a strong opinion formed on this question yet!

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
1102899312  
Powered by Datasette · Queries took 1.345ms · About: github-to-sqlite