home / github

Menu
  • GraphQL API

issue_comments

Table actions
  • GraphQL API for issue_comments

14 rows where issue = 1125297737

✎ View and edit SQL

This data as json, CSV (advanced)

Suggested facets: user, author_association, created_at (date), updated_at (date)

id ▼ html_url issue_url node_id user created_at updated_at author_association body reactions issue performed_via_github_app
1030901189 https://github.com/simonw/sqlite-utils/issues/402#issuecomment-1030901189 https://api.github.com/repos/simonw/sqlite-utils/issues/402 IC_kwDOCGYnMM49ck3F simonw 9599 2022-02-06T19:48:36Z 2022-02-06T19:48:52Z OWNER From [that thread](https://github.com/simonw/sqlite-utils/issues/399#issuecomment-1030739566), two extra ideas which it may be possible to support in a single implementation: ```python from sqlite_utils.conversions import LongitudeLatitude db["places"].insert( { "name": "London", "lng": -0.118092, "lat": 51.509865, }, conversions={"point": LongitudeLatitude("lng", "lat")}, ) ``` And ```python db["places"].insert( { "name": "London", "point": LongitudeLatitude(-0.118092, 51.509865) } ) ``` {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} Advanced class-based `conversions=` mechanism 1125297737  
1030901853 https://github.com/simonw/sqlite-utils/issues/402#issuecomment-1030901853 https://api.github.com/repos/simonw/sqlite-utils/issues/402 IC_kwDOCGYnMM49clBd simonw 9599 2022-02-06T19:52:10Z 2022-02-06T19:52:10Z OWNER So the key idea here is to introduce a new abstract base class, `Conversion`, which has the following abilities: - Can wrap one or more Python values (if called using the constructor) such that the `.insert_all()` method knows how to transform those into a format that can be included in an insert - something like `GeomFromText(?, 4326)` with input `POINT(-0.118092 51.509865)` - Can be passed to `conversions={"point": LongitudeLatitude}` in a way that then knows to apply that conversion to every value in the `"point"` key of the data being inserted. - Maybe also extend `conversions=` to allow the definition of additional keys that use as input other rows? That's the `conversions={"point": LongitudeLatitude("lng", "lat")}` example above - it may not be possible to get this working with the rest of the design though. {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} Advanced class-based `conversions=` mechanism 1125297737  
1030902102 https://github.com/simonw/sqlite-utils/issues/402#issuecomment-1030902102 https://api.github.com/repos/simonw/sqlite-utils/issues/402 IC_kwDOCGYnMM49clFW simonw 9599 2022-02-06T19:53:34Z 2022-02-08T07:40:34Z OWNER I like the idea that the contract for `Conversion` (or rather for its subclasses) is that it can wrap a Python value and then return both the SQL fragment - e.g. `GeomFromText(?, 4326)` - and the values that should be used as the SQL parameters. {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} Advanced class-based `conversions=` mechanism 1125297737  
1030904948 https://github.com/simonw/sqlite-utils/issues/402#issuecomment-1030904948 https://api.github.com/repos/simonw/sqlite-utils/issues/402 IC_kwDOCGYnMM49clx0 simonw 9599 2022-02-06T20:09:42Z 2022-02-08T07:40:44Z OWNER I think this is the code that needs to become aware of this system: https://github.com/simonw/sqlite-utils/blob/fea8c9bcc509bcae75e99ae8870f520103b9aa58/sqlite_utils/db.py#L2453-L2469 There's an earlier branch that runs for upserts which needs to be modified too: https://github.com/simonw/sqlite-utils/blob/fea8c9bcc509bcae75e99ae8870f520103b9aa58/sqlite_utils/db.py#L2417-L2440 {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} Advanced class-based `conversions=` mechanism 1125297737  
1031779460 https://github.com/simonw/sqlite-utils/issues/402#issuecomment-1031779460 https://api.github.com/repos/simonw/sqlite-utils/issues/402 IC_kwDOCGYnMM49f7SE eyeseast 25778 2022-02-07T18:24:56Z 2022-02-07T18:24:56Z CONTRIBUTOR I wonder if there's any overlap with the goals here and the `sqlite3` module's concept of adapters and converters: https://docs.python.org/3/library/sqlite3.html#sqlite-and-python-types I'm not sure that's _exactly_ what we're talking about here, but it might be a parallel with some useful ideas to borrow. {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} Advanced class-based `conversions=` mechanism 1125297737  
1031787865 https://github.com/simonw/sqlite-utils/issues/402#issuecomment-1031787865 https://api.github.com/repos/simonw/sqlite-utils/issues/402 IC_kwDOCGYnMM49f9VZ simonw 9599 2022-02-07T18:33:27Z 2022-02-07T18:33:27Z OWNER Hah, that's interesting - I've never used that mechanism before so it wasn't something that came to mind. They seem to be using a pretty surprising trick there that takes advantage of SQLite allowing you to define a column "type" using a made-up type name, which you can then introspect later. {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} Advanced class-based `conversions=` mechanism 1125297737  
1031791783 https://github.com/simonw/sqlite-utils/issues/402#issuecomment-1031791783 https://api.github.com/repos/simonw/sqlite-utils/issues/402 IC_kwDOCGYnMM49f-Sn eyeseast 25778 2022-02-07T18:37:40Z 2022-02-07T18:37:40Z CONTRIBUTOR I've never used it either, but it's interesting, right? Feel like I should try it for something. I'm trying to get my head around how this conversions feature might work, because I really like the idea of it. {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} Advanced class-based `conversions=` mechanism 1125297737  
1032294365 https://github.com/simonw/sqlite-utils/issues/402#issuecomment-1032294365 https://api.github.com/repos/simonw/sqlite-utils/issues/402 IC_kwDOCGYnMM49h4_d simonw 9599 2022-02-08T07:32:09Z 2022-02-08T07:34:41Z OWNER I have an idea for how that third option could work - the one that creates a new column using values from the existing ones: ```python db["places"].insert( { "name": "London", "lng": -0.118092, "lat": 51.509865, }, conversions={"point": LongitudeLatitude("lng", "lat")}, ) ``` How about specifying that the values in that `conversion=` dictionary can be: - A SQL string fragment (as currently implemented) - A subclass of `Conversion` as described above - Or... a callable function that takes the row as an argument and returns either a `Conversion` subclass instance or a literal value to be jnserted into the database (a string, int or float) Then you could do this: ```python db["places"].insert( { "name": "London", "lng": -0.118092, "lat": 51.509865, }, conversions={ "point": lambda row: LongitudeLatitude( row["lng"], row["lat"] ) } ) ``` Something I really like about this is that it expands the abilities of `conversions=` beyond the slightly obscure need to customize the SQL fragment into something that can solve other data insertion cleanup problems too. {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} Advanced class-based `conversions=` mechanism 1125297737  
1032296717 https://github.com/simonw/sqlite-utils/issues/402#issuecomment-1032296717 https://api.github.com/repos/simonw/sqlite-utils/issues/402 IC_kwDOCGYnMM49h5kN simonw 9599 2022-02-08T07:35:46Z 2022-02-08T07:35:46Z OWNER I'm going to write the documentation for this first, before the implementation, so I can see if it explains cleanly enough that the design appears to be sound. {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} Advanced class-based `conversions=` mechanism 1125297737  
1032732242 https://github.com/simonw/sqlite-utils/issues/402#issuecomment-1032732242 https://api.github.com/repos/simonw/sqlite-utils/issues/402 IC_kwDOCGYnMM49jj5S eyeseast 25778 2022-02-08T15:26:59Z 2022-02-08T15:26:59Z CONTRIBUTOR What if you did something like this: ```python class Conversion: def __init__(self, *args, **kwargs): "Put whatever settings you need here" def python(self, row, column, value): # not sure on args here "Python step to transform value" return value def sql(self, row, column, value): "Return the actual sql that goes in the insert/update step, and maybe params" # value is the return of self.python() return value, [] ``` This way, you're always passing an instance, which has methods that do the conversion. (Or you're passing a SQL string, as you would now.) The `__init__` could take column names, or SRID, or whatever other setup state you need per row, but the row is getting processed with the `python` and `sql` methods (or whatever you want to call them). This is pretty rough, so do what you will with names and args and such. You'd then use it like this: ```python # subclass might be unneeded here, if methods are present class LngLatConversion(Conversion): def __init__(self, x="longitude", y="latitude"): self.x = x self.y = y def python(self, row, column, value): x = row[self.x] y = row[self.y] return x, y def sql(self, row, column, value): # value is now a tuple, returned above s = "GeomFromText(POINT(? ?))" return s, value table.insert_all(rows, conversions={"point": LngLatConversion("lng", "lat"))} ``` I haven't thought through all the implementation details here, and it'll probably break in ways I haven't foreseen, but wanted to get this idea out of my head. Hope it helps. {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} Advanced class-based `conversions=` mechanism 1125297737  
1033366312 https://github.com/simonw/sqlite-utils/issues/402#issuecomment-1033366312 https://api.github.com/repos/simonw/sqlite-utils/issues/402 IC_kwDOCGYnMM49l-so simonw 9599 2022-02-09T05:28:11Z 2022-02-09T07:28:48Z OWNER My hunch is that the case where you want to consider input from more than one column will actually be pretty rare - the only case I can think of where I would want to do that is for latitude/longitude columns - everything else that I'd want to use it for (which admittedly is still mostly SpatiaLite stuff) works against a single value. The reason I'm leaning towards using the constructor for the values is that I really like the look of this variant for common conversions: ```python db["places"].insert( { "name": "London", "boundary": GeometryFromGeoJSON({...}) } ) ``` {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} Advanced class-based `conversions=` mechanism 1125297737  
1033428967 https://github.com/simonw/sqlite-utils/issues/402#issuecomment-1033428967 https://api.github.com/repos/simonw/sqlite-utils/issues/402 IC_kwDOCGYnMM49mN_n simonw 9599 2022-02-09T07:25:44Z 2022-02-09T07:28:11Z OWNER The CLI version of this could perhaps look like this: sqlite-utils insert a.db places places.json \ --conversion boundary GeometryGeoJSON This will treat the boundary key as GeoJSON. It's equivalent to passing `conversions={"boundary": geometryGeoJSON}` The combined latitude/longitude case here can be handled by combining this with the existing `--convert` mechanism. Any `Conversion` subclass will be available to the CLI in this way. {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} Advanced class-based `conversions=` mechanism 1125297737  
1035057014 https://github.com/simonw/sqlite-utils/issues/402#issuecomment-1035057014 https://api.github.com/repos/simonw/sqlite-utils/issues/402 IC_kwDOCGYnMM49sbd2 eyeseast 25778 2022-02-10T15:30:28Z 2022-02-10T15:30:40Z CONTRIBUTOR Yeah, the CLI experience is probably where any kind of multi-column, configured setup is going to fall apart. Sticking with GIS examples, one way I might think about this is using the [fiona CLI](https://fiona.readthedocs.io/en/latest/cli.html): ```sh # assuming a database is already created and has SpatiaLite fio cat boundary.shp | sqlite-utils insert boundaries --conversion geometry GeometryGeoJSON - ``` Anyway, very interested to see where you land here. {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} Advanced class-based `conversions=` mechanism 1125297737  
1041325398 https://github.com/simonw/sqlite-utils/issues/402#issuecomment-1041325398 https://api.github.com/repos/simonw/sqlite-utils/issues/402 IC_kwDOCGYnMM4-EV1W psychemedia 82988 2022-02-16T10:12:48Z 2022-02-16T10:18:55Z NONE > My hunch is that the case where you want to consider input from more than one column will actually be pretty rare - the only case I can think of where I would want to do that is for latitude/longitude columns Other possible pairs: unconventional date/datetime and timezone pairs eg `2022-02-16::17.00, London`; or more generally, numerical value and unit of measurement pairs (eg if you want to cast into and out of different measurement units using packages like `pint`) or currencies etc. Actually, in that case, I guess you may be presenting things that are unit typed already, and so a conversion would need to parse things into an appropriate, possibly two column `value, unit` format. {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} Advanced class-based `conversions=` mechanism 1125297737  

Advanced export

JSON shape: default, array, newline-delimited, object

CSV options:

CREATE TABLE [issue_comments] (
   [html_url] TEXT,
   [issue_url] TEXT,
   [id] INTEGER PRIMARY KEY,
   [node_id] TEXT,
   [user] INTEGER REFERENCES [users]([id]),
   [created_at] TEXT,
   [updated_at] TEXT,
   [author_association] TEXT,
   [body] TEXT,
   [reactions] TEXT,
   [issue] INTEGER REFERENCES [issues]([id])
, [performed_via_github_app] TEXT);
CREATE INDEX [idx_issue_comments_issue]
                ON [issue_comments] ([issue]);
CREATE INDEX [idx_issue_comments_user]
                ON [issue_comments] ([user]);
Powered by Datasette · Queries took 16.526ms · About: simonw/datasette-graphql