Redesign default .json format

The default JSON just isn't right. I find myself using ?_shape=array for almost everything I build against the API.

Here's the default JSON at the moment: https://latest.datasette.io/fixtures/compound_three_primary_keys.json?_size=2
{
  "database": "fixtures",
  "table": "compound_three_primary_keys",
  "is_view": false,
  "human_description_en": "",
  "rows": [
    [
      "a",
      "a",
      "a",
      "a-a-a"
    ],
    [
      "a",
      "a",
      "b",
      "a-a-b"
    ]
  ],
  "truncated": false,
  "filtered_table_rows_count": 1001,
  "expanded_columns": [],
  "expandable_columns": [],
  "columns": [
    "pk1",
    "pk2",
    "pk3",
    "content"
  ],
  "primary_keys": [
    "pk1",
    "pk2",
    "pk3"
  ],
  "units": {},
  "query": {
    "sql": "select pk1, pk2, pk3, content from compound_three_primary_keys order by pk1, pk2, pk3 limit 3",
    "params": {}
  },
  "facet_results": {},
  "suggested_facets": [
    {
      "name": "pk1",
      "toggle_url": "http://latest.datasette.io/fixtures/compound_three_primary_keys.json?_size=2&_facet=pk1"
    },
    {
      "name": "pk2",
      "toggle_url": "http://latest.datasette.io/fixtures/compound_three_primary_keys.json?_size=2&_facet=pk2"
    },
    {
      "name": "pk3",
      "toggle_url": "http://latest.datasette.io/fixtures/compound_three_primary_keys.json?_size=2&_facet=pk3"
    }
  ],
  "next": "a,a,b",
  "next_url": "http://latest.datasette.io/fixtures/compound_three_primary_keys.json?_size=2&_next=a%2Ca%2Cb",
  "query_ms": 17.56119728088379,
  "source": "tests/fixtures.py",
  "source_url": "https://github.com/simonw/datasette/blob/master/tests/fixtures.py",
  "license": "Apache License 2.0",
  "license_url": "https://github.com/simonw/datasette/blob/master/LICENSE"
}

There's a lot of stuff in there. This increases the risk that future minor changes might break existing API consumers.
It returns rows as a list of lists of values, and expects you to correlate these with the list of columns. I originally designed it like this because I thought this was a more efficient representation than repeating the column names in a dictionary for every row. With hindsight this was a bad optimization - I always use ?shape=array because it's more convenient, and gzip encoding of the response means there's no bandwidth saving. Users who want that efficiency should request it using a custom ?_shape=.

https://latest.datasette.io/fixtures/compound_three_primary_keys.json?_size=2&_shape=array returns this:
[
  {
    "pk1": "a",
    "pk2": "a",
    "pk3": "a",
    "content": "a-a-a"
  },
  {
    "pk1": "a",
    "pk2": "a",
    "pk3": "b",
    "content": "a-a-b"
  }
]

There's one big problem with this format: it doesn't provide any space for pagination information.

Another idea: the default output could be the list of dicts:
[
  {
    "pk1": "a",
    "pk2": "a",
    "pk3": "a",
    "content": "a-a-a"
  },
  ...
]

BUT... I could include pagination information in the HTTP headers - as seen in the WordPress REST API or the GitHub API:
~ % curl -s -i 'https://api.github.com/repos/simonw/datasette/commits' | head -n 40
HTTP/1.1 200 OK
server: GitHub.com
date: Fri, 11 Sep 2020 21:37:46 GMT
content-type: application/json; charset=utf-8
status: 200 OK
cache-control: public, max-age=60, s-maxage=60
vary: Accept, Accept-Encoding, Accept, X-Requested-With
etag: W/"71c99379743513394e880c6306b66bf9"
last-modified: Fri, 11 Sep 2020 21:32:54 GMT
x-github-media-type: github.v3; format=json
link: <https://api.github.com/repositories/107914493/commits?page=2>; rel="next", <https://api.github.com/repositories/107914493/commits?page=44>; rel="last"
access-control-expose-headers: ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, Deprecation, Sunset
access-control-allow-origin: *
strict-transport-security: max-age=31536000; includeSubdomains; preload
x-frame-options: deny
x-content-type-options: nosniff
x-xss-protection: 1; mode=block
referrer-policy: origin-when-cross-origin, strict-origin-when-cross-origin
content-security-policy: default-src 'none'
X-Ratelimit-Limit: 60
X-Ratelimit-Remaining: 55
X-Ratelimit-Reset:<PHONE_NUMBER>
X-Ratelimit-Used: 5
Accept-Ranges: bytes
Content-Length: 118240
X-GitHub-Request-Id: EC76:0EAD:313F40:5291A4:5F5BEE37

[
  {
    "sha": "d02f6151dae073135a22d0123e8abdc6cbef7c50",
    "node_id": "MDY6Q29tbWl0MTA3OTE0NDkzOmQwMmY2MTUxZGFlMDczMTM1YTIyZDAxMjNlOGFiZGM2Y2JlZjdjNTA=",
    "commit": {

I'm going to hack together a preview of this in a branch and deploy it somewhere so people can see what I've got planned. Much easier to evaluate a working prototype than static examples.

(I think I may have been over-thinking the details of this is for a couple of years now.)

I'd like to revisit the idea of using ?_extra=x to opt-in to extra blocks of JSON, from #262

Would it be so bad if the default format had a "rows" key containing the array of rows? Maybe it wouldn't. The reason I always use ?_shape=array is because I want an array of objects, rather than an array of arrays that I have to match up again with their columns.

Plan: release a new release of Datasette (probably 0.49) with the new JSON API design, but provide a plugin called something like datasette-api-0-48 which runs as ASGI wrapping middleware and internally rewrites incoming requests to e.g. /db/table.json to behave if they have the ?_extra= params on them necessary to produce the 0.48 version of the JSON.
Anyone who has built applications against 0.48 can install that plugin.

Since the total count can be expensive to calculate, I'm inclined to make that an opt-in extra.
Based on that, the default JSON shape could look something like this:
{
  "rows": [{"id": 1}, {"id": 2}],
  "next": "2",
  "next_url": "/db/table?_next=2"
}

Should that default also include "columns" as a list of strings? That would be duplicate data of the keys in the "rows" list of objects, and I've never found myself wanting it in my own code - so I'm going to say no.

Maybe .jsonfull becomes a new renderer that returns ALL of the defined ?_extra= blocks.

The core issue that I keep reconsidering is whether the default .json representation should be an object or a list.
Arguments in favour of a list:

It's what I always want. Almost all of the code that I've written against the API myself uses ?_shape=array.
It's really easy to use. You can pipe it to e.g. sqlite-utils insert, you can load it into JavaScript without thinking about it.

Arguments against:

Nowhere to put pagination or total counts. I added pagination to the link: HTTP header in #1014 (inspired by the WordPress and GitHub APIs) but I haven't solved for total count, and there's other stuff that's useful like "truncated": true to indicate that more than 1000 results were returned and they were truncated.
An array is inherently non-extensible: if the root item is an object it's easy to add new features to it in a backwards-compatible way in the future. An array is a fixed format.

But maybe that last point is a positive? It ensures the default .json format remains completely predictable forever.
If .json DID default to an array of objects, the ?_shape= argument could still be used to get back alternative formats.
Maybe .json?_extra=total changes the shape of that default to be this instead:
{
  "rows": [{"id": 1}, {"id": 2}],
  "total": 104
}

The thing I care about most though is next_url. That could be provided like so:
.json?_extra=total&_extra=next - alternative syntax .json?_extra=total,next:
{
  "rows": [{"id": 1}, {"id": 2}],
  "total": 104,
  "next": "2",
  "next_url": "/db/table.json?_extra=total&_extra=next&_next=2"
}

This is feeling a bit verbose for a common combination though.

I'm going to prototype what it would look like if the default shape was a list of objects and ?_extra= turns that into an object with a rows key, in a plugin.

Building this plugin reminded me of an oddity of the register_output_renderer() plugin hook: one of the arguments that can be passed to it is data, which is the default internal data structure created by Datasette - but I deliberately avoided documenting that on https://docs.datasette.io/en/stable/plugin_hooks.html#register-output-renderer-datasette because it's not a stable interface.
That's not ideal. I'd like custom renderers to be able to access this data to get at things like suggested facets, on an opt-in basis.
So maybe that kind of stuff is re-implemented as "extras" which are awaitable callables - then renderer plugins can call the extras that they need to as part of their execution.

Here's the datasette-json-preview plugin I'll be using to experiment with different formats: https://github.com/simonw/datasette-json-preview

I vote against headers. It has a lot of strikes against it: poor discoverability, new developers often don’t know how to use them, makes CORS harder, makes it hard to use eg with JQ, needs ad Hoc specification for each bit of metadata, etc.
The only advantage of headers is that you don’t need to do .rows, but that’s actually good as a data validation step anyway—if .rows is missing assume there’s an error and do your error handling path instead of parsing the rest.

Great point about CORS, I hadn't considered that.
I think I'm going to keep the Link: header (added in #1014) because I quite enjoy using it with GitHub and WordPress, but I'm not going to have it be the default way of doing pagination. For the default shape I'm leaning towards this:
{
  "total": 36,
  "rows": [{"id": 1, "name": "Cleo"}],
  "next_url": "https://latest-with-plugins.datasette.io/fixtures/facetable.json?_next=5"
}

So three keys: total, rows and next_url. Then extra keys can be added using ?_extra= with various named bundles.

I'll update datasette-json-preview with that now.

OK, https://latest-with-plugins.datasette.io/ is running that now - e.g. https://latest-with-plugins.datasette.io/fixtures/roadside_attractions.json-preview or https://latest-with-plugins.datasette.io/fixtures/compound_three_primary_keys.json-preview

I think this all comes down to how the ?_extras= mechanism works, as first hinted at in a30c5b220c15360d575e94b0e67f3255e120b916 (see commit message) when I added this long-forgotten undocumented feature: https://latest.datasette.io/fixtures/attraction_characteristic/2.json?_extras=foreign_key_tables
Extras need to be able to execute additional SQL, since that would solve the problem we have now where the expensive "suggested facets" code runs on all .json output even when its results are not being shown.

This should be the key focus for Datasette 0.52.

https://latest-with-plugins.datasette.io/fixtures/roadside_attraction_characteristics/1.json-preview returns a 500 error at the moment - a KeyError on 'filtered_table_rows_count'.

In building https://latest-with-plugins.datasette.io/github/issue_comments.Notebook?_labels=on I discovered the following patterns for importing data into both Pandas and Observable/d3:
import pandas
df = pandas.read_json("https://latest-with-plugins.datasette.io/github/issue_comments.Notebook?_labels=on&_format=json&_shape=array")

And:
d3 = require("d3@5")
rows = d3.json("https://latest-with-plugins.datasette.io/github/issue_comments.Notebook?_labels=on&_format=json&_shape=array")

Once again I find myself torn on the best possible default. A list of JSON objects is instantly compatible with both pandas.read_json() and d3.json() - but it leaves nowhere to put the extra information like pagination and suchlike!
Even given this I still think the correct default is an object with "rows", "total" and "next_url" keys. I should commit to that and implement it - this thought exercise has been running for far too long.

In building https://latest-with-plugins.datasette.io/github/issue_comments.Notebook?_labels=on I discovered the following patterns for importing data into both Pandas and Observable/d3:
import pandas
df = pandas.read_json("https://latest-with-plugins.datasette.io/github/issue_comments.Notebook?_labels=on&_format=json&_shape=array")

And:
d3 = require("d3@5")
rows = d3.json("https://latest-with-plugins.datasette.io/github/issue_comments.Notebook?_labels=on&_format=json&_shape=array")

Once again I find myself torn on the best possible default. A list of JSON objects is instantly compatible with both pandas.read_json() and d3.json() - but it leaves nowhere to put the extra information like pagination and suchlike!
Even given this I still think the correct default is an object with "rows", "total" and "next_url" keys. I should commit to that and implement it - this thought exercise has been running for far too long.

I'm leaning back in the direction of a flat JSON array of objects as the default - this:
/fixtures/roadside_attractions.json
Would return:
[
    {
      "pk": 1,
      "name": "The Mystery Spot",
      "address": "465 Mystery Spot Road, Santa Cruz, CA 95065",
      "latitude": 37.0167,
      "longitude": -122.0024
    },
    {
      "pk": 2,
      "name": "Winchester Mystery House",
      "address": "525 South Winchester Boulevard, San Jose, CA 95128",
      "latitude": 37.3184,
      "longitude": -121.9511
    },
    {
      "pk": 3,
      "name": "Burlingame Museum of PEZ Memorabilia",
      "address": "214 California Drive, Burlingame, CA 94010",
      "latitude": 37.5793,
      "longitude": -122.3442
    },
    {
      "pk": 4,
      "name": "Bigfoot Discovery Museum",
      "address": "5497 Highway 9, Felton, CA 95018",
      "latitude": 37.0414,
      "longitude": -122.0725
    }
]

To get the version that includes pagination information you would use the ?_extra= parameter. For example:
/fixtures/roadside_attractions.json?_extra=total&_extra=next_url
{
  "rows": [
    {
      "pk": 1,
      "name": "The Mystery Spot",
      "address": "465 Mystery Spot Road, Santa Cruz, CA 95065",
      "latitude": 37.0167,
      "longitude": -122.0024
    },
    {
      "pk": 2,
      "name": "Winchester Mystery House",
      "address": "525 South Winchester Boulevard, San Jose, CA 95128",
      "latitude": 37.3184,
      "longitude": -121.9511
    },
    {
      "pk": 3,
      "name": "Burlingame Museum of PEZ Memorabilia",
      "address": "214 California Drive, Burlingame, CA 94010",
      "latitude": 37.5793,
      "longitude": -122.3442
    },
    {
      "pk": 4,
      "name": "Bigfoot Discovery Museum",
      "address": "5497 Highway 9, Felton, CA 95018",
      "latitude": 37.0414,
      "longitude": -122.0725
    }
  ],
  "total": 4,
  "next_url": null
}

ANY usage of the ?_extra= parameter would turn the list into an object with a "rows" key.
Opting in to the total is nice because it's actually expensive to run a count, so only doing a count if the user requests it feels good.
But... having to add ?_extra=total&_extra=next_url for the common case of wanting both the total count and the URL to get the next page of results is a bit verbose. So maybe support aliases, like ?_extra=paginated which is a shortcut for ?_extra=total&_extra=next_url?

Also how would you opt out of returning the "rows" key? I sometimes want to do this - if I want to get back just the count or just the facets for example.
Some options:

/fixtures/roadside_attractions.json?_extra=total&_extra=-rows
/fixtures/roadside_attractions.json?_extra=total&_skip=rows
/fixtures/roadside_attractions.json?_extra=total&_size=0

I quite like that last one with ?_size=0.

Having shortcuts means I could support ?_extra=all for returning ALL possible keys.

Demos:

https://latest-with-plugins.datasette.io/github/commits.json-preview
https://latest-with-plugins.datasette.io/github/commits.json-preview?_extra=next_url
https://latest-with-plugins.datasette.io/github/commits.json-preview?_extra=total
https://latest-with-plugins.datasette.io/github/commits.json-preview?_extra=next_url&_extra=total

Am I saying you won't get back a key in the response unless you explicitly request it, either by name or by specifying a bundle of extras (e.g. all or paginated)?
The "truncated": true key that tells you that your arbitrary query returned more than X results but was truncated is pretty important, do I really want people to have to opt-in to that one?
Also: having bundles like all or paginated live in the same namespace as single keys like next_url or total is a little odd - you can't tell by looking at them if they'll add a key called all or if they'll add a bunch of other stuff.
Maybe bundles could be prefixed with something, perhaps an underscore? ?_extra=_all and ?_extra=_paginated for example.

I think I want to support ?_extra=next_url,total in addition to ?_extra=next_url&_extra=total - partly because it's easier to type, and also because I know there exist URL handling library that don't know how to handle the same parameter multiple times (though they're going to break against Datasette already, so it's not a big deal).

This design should be influenced by how it's implemented.
One implementation that could be nice is that each of the keys that can be requested - next_url, total etc - maps to an async def function which can do the work. So that expensive count(*) will only be executed by the async def total function if it is requested.
This raises more questions: Both next and next_url work off the same underlying data, so if they are both requested can we re-use the work that next does somehow? Maybe by letting these functions depend on each other (so next_url() knows to first call next(), but only if it hasn't been called already.
I think I need to flesh out the full default collection of ?_extra= parameters in order to design how they will work under the hood.

A really exciting opportunity this opens up is for parallel execution - the facets() and suggested_facets() and total() async functions could be called in parallel, which could speed things up if I'm confident the SQLite thread pool can execute on multiple CPU cores (it should be able to because the Python sqlite3 module releases the GIL while it's executing C code).

The only advantage of headers is that you don’t need to do .rows, but that’s actually good as a data validation step anyway—if .rows is missing assume there’s an error and do your error handling path instead of parsing the rest.

This is something I've not thought very hard about. If there's an error, I need to return a top-level object, not a top-level array, so I can provide details of the error.
But this means that client code will have to handle this difference - it will have to know that the returned data can be array-shaped if nothing went wrong, and object-shaped if there's an error.
The HTTP status code helps here - calling client code can know that a 200 status code means there will be an array, but an error status code means an object.
If developers really hate that the shape could be different, they can always use ?_extra=next to ensure that the top level item is an object whether or not an error occurred. So I think this is OK.

I just noticed that https://latest-with-plugins.datasette.io/github/commits.json-preview?_extra=total&_size=0&_trace=1 executes 35 SQL queries at the moment! A great reminder that a big improvement from this change will be a reduction in queries through not calculating things like suggested facets unless they are explicitly requested.

I think it’s a good idea if the top level item of the response JSON is always an object, rather than an array, at least as the default. Mainly because it allows you to add extra keys in a backwards-compatible way. Also just seems more expected somehow.
The API design guidance for the UK government also recommends this: https://www.gov.uk/guidance/gds-api-technical-and-data-standards#use-json
I also strongly dislike having versioned APIs (eg with a /v1/ path prefix, as it invariably means that old versions stop working at some point, even though the bit of the API you’re using might not have changed at all.

Another option: .json always returns an object with a list of keys that gets increased through adding ?_extra= parameters.
.jsona always returns a JSON array of objects
I had something similar to this in Datasette a few years ago - a .jsono extension, which still redirects to the shape=array version.

Minor suggestion: rename size query param to limit, to better reflect that it’s a maximum number of rows returned rather than a guarantee of getting that number, and also for consistency with the SQL keyword?
I like the idea of specifying a limit of 0 if you don’t want any rows data - and returning an empty array under the rows key seems fine.
Have you given any thought as to whether to pretty print (format with spaces) the output or not? Can be useful for debugging/exploring in a browser or other basic tools which don’t parse the JSON. Could be default (can’t be much bigger with gzip?) or opt-in.

(I started a thread on Twitter about this: https://twitter.com/simonw/status/1363220355318358016)

Minor suggestion: rename size query param to limit, to better reflect that it’s a maximum number of rows returned rather than a guarantee of getting that number, and also for consistency with the SQL keyword?

The problem there is that ?_size=x isn't actually doing the same thing as the SQL limit keyword. Consider this query:
https://latest-with-plugins.datasette.io/github?sql=select+*+from+commits - select * from commits
Datasette returns 1,000 results, and shows a "Custom SQL query returning more than 1,000 rows" message at the top. That's the size kicking in - I only fetch the first 1,000 results from the cursor to avoid exhausting resources. In the JSON version of that at https://latest-with-plugins.datasette.io/github.json?sql=select+*+from+commits there's a "truncated": true key to let you know what happened.
I find myself using ?_size=2 against Datasette occasionally if I know the rows being returned are really big and I don't want to load 10+MB of HTML.
This is only really a concern for arbitrary SQL queries though - for table pages such as https://latest-with-plugins.datasette.io/github/commits?_size=10 adding ?_size=10 actually puts a limit 10 on the underlying SQL query.

... though thinking about this further, I could re-implement the select * from commits (but only return a max of 10 results) feature using a nested select * from (select * from commits) limit 10 query.

Have you given any thought as to whether to pretty print (format with spaces) the output or not? Can be useful for debugging/exploring in a browser or other basic tools which don’t parse the JSON. Could be default (can’t be much bigger with gzip?) or opt-in.

Adding a ?_pretty=1 option that does that is a great idea, I'm filing a ticket for it: #1237

Yet another option: support a ?_path=x option which returns a nested path from the result. So you could do this:
/github/commits.json?_path=rows - to get back a top-level array pulled from the "rows" key.

I think it’s a good idea if the top level item of the response JSON is always an object, rather than an array, at least as the default.

I agree it is more predictable if the top level item is an object with a rows or data object that contains an array of data, which then allows for other top-level meta data.
I can see the argument for removing this and just using an array for convenience - but I think that's OK as an option (as you have now).
Rather than have lots of top-level keys you could have a "meta" object to contain non-data stuff. You could use something like "links" for API endpoint URLs (or use a standard like HAL). Which would then leave the top level a bit cleaner - if that's what you what.
Have you had much feedback from users who use the Datasette API a lot?

OK, I'm back to the "top level object as the default" side of things now - it's pretty much unanimous at this point, and it's certainly true that it's not a decision you'll even regret.

Another benefit to default:object - I could include a key that shows a list of available extras. I could then use that to power an interactive API explorer.

@simonw:

The problem there is that ?_size=x isn't actually doing the same thing as the SQL limit keyword.

Interesting! Although I don't think it matters too much what the underlying implementation is - I more meant that limit is familiar to developers conceptually as "up to and including this number, if they exist", whereas "size" is potentially more ambiguous. However, it's probably no big deal either way.

I'm going to keep ?_shape=array working on the assumption that many existing uses of the Datasette API are already using that option, so it would be nice not to break them.

OK, I'm ready to start working on this today.
I'm going to go with a default representation that looks like this:
{
    "rows": [
        {"id": 1, "name": "One"},
        {"id": 2, "name": "Two"}
    ],
    "next_url": null,
}

Note that there's no count - all it provides is the current selection of results and an indication as to how the next can be retrieved (null if there are no more results).
I'll implement ?_extra= to provide everything else.

I'm going to take another look at this:

https://github.com/simonw/datasette/issues/878

It turned out the most significant part of this change - switching from an array of arrays to an array of objects for the "rows" key - was really easy: Datasette already had a ?_shape=arrays v.s. ?_shape=objects mechanism, so I switched which one was the default in https://github.com/simonw/datasette/commit/234230e59574ccb8d8a24c45ccd325f725812377

https://latest.datasette.io/fixtures/compound_three_primary_keys.json?_size=2 now shows the new default:
{
  "database": "fixtures",
  "table": "compound_three_primary_keys",
  "is_view": false,
  "human_description_en": "",
  "rows": [
    {
      "pk1": "a",
      "pk2": "a",
      "pk3": "a",
      "content": "a-a-a"
    },
    {
      "pk1": "a",
      "pk2": "a",
      "pk3": "b",
      "content": "a-a-b"
    }
  ],

The old format can be had like this: https://latest.datasette.io/fixtures/compound_three_primary_keys.json?_size=2&_shape=arrays

New thinking on the trimmed-down default. Previously I was going to use "row" and "next_url" - I now want to do this instead:
{
  "rows": [
    {
      "pk1": "a",
      "pk2": "a",
      "pk3": "a",
      "content": "a-a-a"
    },
    {
      "pk1": "a",
      "pk2": "a",
      "pk3": "b",
      "content": "a-a-b"
    }
  ],
  "next": "a,a,b"
}

If there isn't a next page it will return "next": null.
This is even more succinct. I'm OK with people having to request next_url if they don't want to construct the new URL themselves.

Here's the so-far updated documentation for this change: https://github.com/simonw/datasette/blob/a2dca62360ad4a961d4c46f68eae41b7d5c7b2c9/docs/json_api.rst#different-shapes

I'm going to rename filtered_table_rows_count to count - to match the SQL count(*) function.

https://latest.datasette.io/fixtures/compound_three_primary_keys.json?_size=2 now returns count:
{
  "database": "fixtures",
  "table": "compound_three_primary_keys",
  "is_view": false,
  "human_description_en": "",
  "rows": [
    {
      "pk1": "a",
      "pk2": "a",
      "pk3": "a",
      "content": "a-a-a"
    },
    {
      "pk1": "a",
      "pk2": "a",
      "pk3": "b",
      "content": "a-a-b"
    }
  ],
  "truncated": false,
  "count": 1001,

Most of this is shipped in https://docs.datasette.io/en/1.0a3/changelog.html#a3-2023-08-09


Consider using patterns for larval motor neurons

Now that dosdp can be run to make terms each release, it might be good to patternise the segment-specific motor neurons of the larva. This would allow easier renaming, generation of more detailed definitions (including soma location as well as muscle location), generation of a wider range of synonyms and better consistency.

todo - patternise A1-7 terms, type Ib and Is parent terms, type II parents(?) - not sure if type II can be defined logically


Vibration Timer Notification

Thanks a lot for your work :) I have tested it with Fedora 29 and it works very well !
Do you think it is possible to implement the timer functionnality ?

Hi, thank you for your feedback, great to hear that the project works with Fedora.
What do you mean by timer functionality? Is that a feature from the original Logitech Windows software? (I never used the Windows version)

I have tested it yesterday on my Windows dual boot.
There is a settings screen where you choose when the remote give you a vibration feedback. For the moment, I don't suceed to start the timer. I will try to test it today.
I think that it is not important to display a timer as it can be done by a lot of other applications. But the remote vibration should be very cool during a conference :)
I have never developed a driver, and never developed with c++, but I will try to read your code to see if I can help. I have a relatively big conference next week :)

The notification with a vibration feedback is indeed a nice feature, but I suspect that this will require connecting to the device directly via libusb and reverse engineering the command that the device expects to trigger a vibration.
Currently the application does read input events from the Linux input system and its devices.
I would like to have this feature, but I will be only able to investigate it in a few weeks - currently it's pretty busy. But contributions are very welcome :)

I have done a wireshark dump and it seems that there is only one message sent via USB for vibration. This message is always the same.
For the moment, I'll try to send it with a lib in python. If it works, I'll share my work.

As a not to myself: https://www.kernel.org/doc/html/v4.13/input/ff.html# (Check if the registered HID device supports this, then vibrations support will be really easy)

Interesting!!! There is an utility to test if it works :) I'll try to test this weekend.

@alocquet Did get around to do your tests?

It seems that the device don't support ff.
I have also try to send HID control commands with C code without success... The packet I have caught with wireshark is sent but the device doesn't vibrate.

@alocquet, did you have success with your python program? What is the USB message that is sent for vibration of the device?

No success... The message is : 21 09 10 02 02 00 07 00
That means :
bmRequestType : 0x21
bRequest : 0x09 (SET_REPORT)
wValue : 0x0210
wIndex : 2
wLength : 7
I managed to

Thank you. Too bad it's not working. I sent a request to the Logitech support, asking if they are willing to share more information about it. I doubt that I will get a reply - but worth a try.

Alright, I had some success :smiley:
The following ruby script works for me and lets the device vibrate via libusb. (With the USB connector)
So in the future I am thinking about a libusb backend for Projecteur that handles the device via libusb instead of the simple version via the linux event system.
#!/usr/bin/env ruby

require 'libusb'
require 'pp'

usb = LIBUSB::Context.new
device = usb.devices(idVendor: 0x46d, idProduct: 0xc53e).first
pp device
pp device.interfaces

begin
  h = device.open
  pp h
  h.auto_detach_kernel_driver = true
  h.claim_interface(device.interfaces[0])
  data=[0x10,0x01,0x09,0x1e,0x03,0xe8,0x75].pack("c*")
  h.control_transfer(bmRequestType: 0x21, bRequest: 0x09, wValue: 0x0210, wIndex: 0x0002, dataOut: data)
ensure
  h.release_interface(device.interfaces[0])
  h.close
end

Good news :tada:  !!!
Where do you have found data values ? When I had made my wireshark record, it was an 0x00 array.

My setup for capturing the USB messages was Windows inside a VM with the Logitech software and Wireshark running on the Linux host.

@jahnf With the above posted ruby code I am getting IO_ERROR. The traceback is
#<LIBUSB::Device 1/13 046d:c53e Logitech USB Receiver ? (HID (01,01), HID (01,02), HID (00,00))>
[#<LIBUSB::Interface 0>, #<LIBUSB::Interface 1>, #<LIBUSB::Interface 2>]
#<LIBUSB::DevHandle:0x0000556012d1b858
 @bulk_transfer=nil,
 @control_transfer=nil,
 @device=
  #<LIBUSB::Device 1/13 046d:c53e Logitech USB Receiver ? (HID (01,01), HID (01,02), HID (00,00))>,
 @interrupt_transfer=nil,
 @pHandle=#<FFI::Pointer address=0x0000556012d278b0>>
Traceback (most recent call last):
	5: from ./test_ruby.rb:17:in `<main>'
	4: from /home/mayank/.gem/ruby/2.5.0/gems/libusb-0.6.4-x86_64-linux/lib/libusb/dev_handle.rb:525:in `control_transfer'
	3: from /home/mayank/.gem/ruby/2.5.0/gems/libusb-0.6.4-x86_64-linux/lib/libusb/dev_handle.rb:544:in `submit_transfer'
	2: from /home/mayank/.gem/ruby/2.5.0/gems/libusb-0.6.4-x86_64-linux/lib/libusb/transfer.rb:246:in `submit_and_wait'
	1: from /home/mayank/.gem/ruby/2.5.0/gems/libusb-0.6.4-x86_64-linux/lib/libusb/transfer.rb:217:in `submit!'
/home/mayank/.gem/ruby/2.5.0/gems/libusb-0.6.4-x86_64-linux/lib/libusb/constants.rb:68:in `raise_error': LIBUSB::ERROR_IO in libusb_submit_transfer (LIBUSB::ERROR_IO)

I am new to ruby. How to find out the line causing error?

I am no Ruby expert, Ruby was just the quickest way for me to create a proof of concept.
Looks like you might not have access rights to the device? (Udev rules) . Did you try as root?

Yes, I have tried as root too.
I was able to find the reason for the problem: If you have run Windows guest (having Logitech official software) in virtualbox in the user session, then even if virtualbox is not running the vibration command fail. The best way to fix is to relogin into gnome session.

Ok. I have found all the data packets for which SpotLight device vibrate. All the data packets are 7 byte long with following content
10 01 09 zz 03 e8 yy

where zz ranges between 10 to 1f and yy can have values between 00 to ff. Changing yy changes the intensity of vibration. However, I was not able to find out the meaning of zz (may be reason for which vibration is done - battery/timer).
The Logitech official software uses zz as 1a during demonstration of vibration intensity.

So there is a way without libusb and there will be no need to attach/reattach the device. One can achieve it with connecting directly to the correct hidraw sub-device. Actually this can even be done without any third party libraries (there is no need for even hidapi) and just using regular open/read/write and some additional defines from <linux/hidraw.h>

Will start working on this issue soon - I planned it for the next minor release.

Vibration timer support for the Logitech Spotlight was added.
Feel free to test and report bugs :smiley:


script/bootstrap fails

Prerequisites

[x] Put an X between the brackets on this line if you have done all of the following:

Reproduced the problem in Safe Mode: https://flight-manual.atom.io/hacking-atom/sections/debugging/#using-safe-mode
Followed all applicable steps in the debugging guide: https://flight-manual.atom.io/hacking-atom/sections/debugging/
Checked the FAQs on the message board for common solutions: https://discuss.atom.io/c/faq
Checked that your issue isn't already filed: https://github.com/issues?utf8=✓&q=is%3Aissue+user%3Aatom
Checked that there is not already an Atom package that provides the described functionality: https://atom.io/packages

Description
env PYTHON=/usr/bin/python2 script/bootstrap fails at Arch Linux. All dependencies are installed.
Steps to Reproduce

Clone atom/atom (or fork) repo.
Run script/bootstrap with env PYTHON=/usr/bin/python2 in fish-shell, or set -x PYTHON /usr/bin/python2, or export PYTHON=/usr/bin/python2 in bash
Get the error

Expected behavior: Script works.
Actual behavior: Script fails.
Reproduces how often: 100%.
Versions
Atom Git master branch.
Additional Information
> bash
$ export PYTHON=/usr/bin/python2
$ script/bootstrap
Node:	v10.1.0
Npm:	v5.3.0
Installing script dependencies
../deps/leveldb/leveldb-1.20/util/hash.cc: In function ‘uint32_t leveldb::Hash(const char*, size_t, uint32_t)’:
../deps/leveldb/leveldb-1.20/util/hash.cc:37:9: warning: this statement may fall through [-Wimplicit-fallthrough=]
       h += static_cast<unsigned char>(data[2]) << 16;
       ~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../deps/leveldb/leveldb-1.20/util/hash.cc:39:5: note: here
     case 2:
     ^~~~
../deps/leveldb/leveldb-1.20/util/hash.cc:40:9: warning: this statement may fall through [-Wimplicit-fallthrough=]
       h += static_cast<unsigned char>(data[1]) << 8;
       ~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../deps/leveldb/leveldb-1.20/util/hash.cc:42:5: note: here
     case 1:
     ^~~~
In file included from ../../nan/nan.h:192,
                 from ../src/batch.cc:3:
../../nan/nan_maybe_43_inl.h: In function ‘Nan::Maybe<bool> Nan::ForceSet(v8::Local<v8::Object>, v8::Local<v8::Value>, v8::Local<v8::Value>, v8::PropertyAttribute)’:
../../nan/nan_maybe_43_inl.h:112:15: error: ‘class v8::Object’ has no member named ‘ForceSet’
   return obj->ForceSet(isolate->GetCurrentContext(), key, value, attribs);
               ^~~~~~~~
In file included from ../src/batch.cc:3:
../../nan/nan.h: In function ‘v8::Local<v8::Value> Nan::MakeCallback(v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*)’:
../../nan/nan.h:835:60: warning: ‘v8::Local<v8::Value> node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*)’ is deprecated: Use MakeCallback(..., async_context) [-Wdeprecated-declarations]
         v8::Isolate::GetCurrent(), target, func, argc, argv);
                                                            ^
In file included from ../src/batch.cc:1:
/home/alex/.node-gyp/10.1.0/include/node/node.h:172:50: note: declared here
                 NODE_EXTERN v8::Local<v8::Value> MakeCallback(
                                                  ^~~~~~~~~~~~
/home/alex/.node-gyp/10.1.0/include/node/node.h:88:42: note: in definition of macro ‘NODE_DEPRECATED’
     __attribute__((deprecated(message))) declarator
                                          ^~~~~~~~~~
In file included from ../src/batch.cc:3:
../../nan/nan.h:835:60: warning: ‘v8::Local<v8::Value> node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*)’ is deprecated: Use MakeCallback(..., async_context) [-Wdeprecated-declarations]
         v8::Isolate::GetCurrent(), target, func, argc, argv);
                                                            ^
In file included from ../src/batch.cc:1:
/home/alex/.node-gyp/10.1.0/include/node/node.h:172:50: note: declared here
                 NODE_EXTERN v8::Local<v8::Value> MakeCallback(
                                                  ^~~~~~~~~~~~
/home/alex/.node-gyp/10.1.0/include/node/node.h:88:42: note: in definition of macro ‘NODE_DEPRECATED’
     __attribute__((deprecated(message))) declarator
                                          ^~~~~~~~~~
In file included from ../src/batch.cc:3:
../../nan/nan.h: In function ‘v8::Local<v8::Value> Nan::MakeCallback(v8::Local<v8::Object>, v8::Local<v8::String>, int, v8::Local<v8::Value>*)’:
../../nan/nan.h:850:62: warning: ‘v8::Local<v8::Value> node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, v8::Local<v8::String>, int, v8::Local<v8::Value>*)’ is deprecated: Use MakeCallback(..., async_context) [-Wdeprecated-declarations]
         v8::Isolate::GetCurrent(), target, symbol, argc, argv);
                                                              ^
In file included from ../src/batch.cc:1:
/home/alex/.node-gyp/10.1.0/include/node/node.h:165:50: note: declared here
                 NODE_EXTERN v8::Local<v8::Value> MakeCallback(
                                                  ^~~~~~~~~~~~
/home/alex/.node-gyp/10.1.0/include/node/node.h:88:42: note: in definition of macro ‘NODE_DEPRECATED’
     __attribute__((deprecated(message))) declarator
                                          ^~~~~~~~~~
In file included from ../src/batch.cc:3:
../../nan/nan.h:850:62: warning: ‘v8::Local<v8::Value> node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, v8::Local<v8::String>, int, v8::Local<v8::Value>*)’ is deprecated: Use MakeCallback(..., async_context) [-Wdeprecated-declarations]
         v8::Isolate::GetCurrent(), target, symbol, argc, argv);
                                                              ^
In file included from ../src/batch.cc:1:
/home/alex/.node-gyp/10.1.0/include/node/node.h:165:50: note: declared here
                 NODE_EXTERN v8::Local<v8::Value> MakeCallback(
                                                  ^~~~~~~~~~~~
/home/alex/.node-gyp/10.1.0/include/node/node.h:88:42: note: in definition of macro ‘NODE_DEPRECATED’
     __attribute__((deprecated(message))) declarator
                                          ^~~~~~~~~~
In file included from ../src/batch.cc:3:
../../nan/nan.h: In function ‘v8::Local<v8::Value> Nan::MakeCallback(v8::Local<v8::Object>, const char*, int, v8::Local<v8::Value>*)’:
../../nan/nan.h:865:62: warning: ‘v8::Local<v8::Value> node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, const char*, int, v8::Local<v8::Value>*)’ is deprecated: Use MakeCallback(..., async_context) [-Wdeprecated-declarations]
         v8::Isolate::GetCurrent(), target, method, argc, argv);
                                                              ^
In file included from ../src/batch.cc:1:
/home/alex/.node-gyp/10.1.0/include/node/node.h:158:50: note: declared here
                 NODE_EXTERN v8::Local<v8::Value> MakeCallback(
                                                  ^~~~~~~~~~~~
/home/alex/.node-gyp/10.1.0/include/node/node.h:88:42: note: in definition of macro ‘NODE_DEPRECATED’
     __attribute__((deprecated(message))) declarator
                                          ^~~~~~~~~~
In file included from ../src/batch.cc:3:
../../nan/nan.h:865:62: warning: ‘v8::Local<v8::Value> node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, const char*, int, v8::Local<v8::Value>*)’ is deprecated: Use MakeCallback(..., async_context) [-Wdeprecated-declarations]
         v8::Isolate::GetCurrent(), target, method, argc, argv);
                                                              ^
In file included from ../src/batch.cc:1:
/home/alex/.node-gyp/10.1.0/include/node/node.h:158:50: note: declared here
                 NODE_EXTERN v8::Local<v8::Value> MakeCallback(
                                                  ^~~~~~~~~~~~
/home/alex/.node-gyp/10.1.0/include/node/node.h:88:42: note: in definition of macro ‘NODE_DEPRECATED’
     __attribute__((deprecated(message))) declarator
                                          ^~~~~~~~~~
In file included from ../src/batch.cc:3:
../../nan/nan.h: In member function ‘v8::Local<v8::Value> Nan::Callback::Call_(v8::Isolate*, v8::Local<v8::Object>, int, v8::Local<v8::Value>*) const’:
../../nan/nan.h:1479:5: warning: ‘v8::Local<v8::Value> node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*)’ is deprecated: Use MakeCallback(..., async_context) [-Wdeprecated-declarations]
     ));
     ^
In file included from ../src/batch.cc:1:
/home/alex/.node-gyp/10.1.0/include/node/node.h:172:50: note: declared here
                 NODE_EXTERN v8::Local<v8::Value> MakeCallback(
                                                  ^~~~~~~~~~~~
/home/alex/.node-gyp/10.1.0/include/node/node.h:88:42: note: in definition of macro ‘NODE_DEPRECATED’
     __attribute__((deprecated(message))) declarator
                                          ^~~~~~~~~~
In file included from ../src/batch.cc:3:
../../nan/nan.h:1479:5: warning: ‘v8::Local<v8::Value> node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*)’ is deprecated: Use MakeCallback(..., async_context) [-Wdeprecated-declarations]
     ));
     ^
In file included from ../src/batch.cc:1:
/home/alex/.node-gyp/10.1.0/include/node/node.h:172:50: note: declared here
                 NODE_EXTERN v8::Local<v8::Value> MakeCallback(
                                                  ^~~~~~~~~~~~
/home/alex/.node-gyp/10.1.0/include/node/node.h:88:42: note: in definition of macro ‘NODE_DEPRECATED’
     __attribute__((deprecated(message))) declarator
                                          ^~~~~~~~~~
In file included from ../src/batch.cc:3:
../../nan/nan.h: In function ‘void Nan::AsyncQueueWorker(Nan::AsyncWorker*)’:
../../nan/nan.h:1732:62: warning: cast between incompatible function types from ‘void (*)(uv_work_t*)’ {aka ‘void (*)(uv_work_s*)’} to ‘uv_after_work_cb’ {aka ‘void (*)(uv_work_s*, int)’} [-Wcast-function-type]
     , reinterpret_cast<uv_after_work_cb>(AsyncExecuteComplete)
                                                              ^
In file included from ../src/batch.cc:5:
../src/database.h: At global scope:
../src/database.h:86:7: warning: unnecessary parentheses in declaration of ‘pendingCloseWorker’ [-Wparentheses]
   void(*pendingCloseWorker);
       ^
In file included from /home/alex/.node-gyp/10.1.0/include/node/node.h:63,
                 from ../src/batch.cc:1:
/home/alex/.node-gyp/10.1.0/include/node/v8.h: In instantiation of ‘void v8::PersistentBase<T>::SetWeak(P*, typename v8::WeakCallbackInfo<P>::Callback, v8::WeakCallbackType) [with P = node::ObjectWrap; T = v8::Object; typename v8::WeakCallbackInfo<P>::Callback = void (*)(const v8::WeakCallbackInfo<node::ObjectWrap>&)]’:
/home/alex/.node-gyp/10.1.0/include/node/node_object_wrap.h:85:78:   required from here
/home/alex/.node-gyp/10.1.0/include/node/v8.h:9227:16: warning: cast between incompatible function types from ‘v8::WeakCallbackInfo<node::ObjectWrap>::Callback’ {aka ‘void (*)(const v8::WeakCallbackInfo<node::ObjectWrap>&)’} to ‘Callback’ {aka ‘void (*)(const v8::WeakCallbackInfo<void>&)’} [-Wcast-function-type]
                reinterpret_cast<Callback>(callback), type);
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/alex/.node-gyp/10.1.0/include/node/v8.h: In instantiation of ‘void v8::PersistentBase<T>::SetWeak(P*, typename v8::WeakCallbackInfo<P>::Callback, v8::WeakCallbackType) [with P = Nan::ObjectWrap; T = v8::Object; typename v8::WeakCallbackInfo<P>::Callback = void (*)(const v8::WeakCallbackInfo<Nan::ObjectWrap>&)]’:
../../nan/nan_object_wrap.h:66:61:   required from here
/home/alex/.node-gyp/10.1.0/include/node/v8.h:9227:16: warning: cast between incompatible function types from ‘v8::WeakCallbackInfo<Nan::ObjectWrap>::Callback’ {aka ‘void (*)(const v8::WeakCallbackInfo<Nan::ObjectWrap>&)’} to ‘Callback’ {aka ‘void (*)(const v8::WeakCallbackInfo<void>&)’} [-Wcast-function-type]
make: *** [leveldown.target.mk:113: Release/obj.target/leveldown/src/batch.o] Error 1
gyp ERR! build error
gyp ERR! stack Error: `make` failed with exit code: 2
gyp ERR! stack     at ChildProcess.onExit (/home/alex/Projects/atom/atom/script/node_modules/npm/node_modules/node-gyp/lib/build.js:258:23)
gyp ERR! stack     at ChildProcess.emit (events.js:182:13)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:235:12)
gyp ERR! System Linux 4.16.9-1-ARCH
gyp ERR! command "/usr/bin/node" "/home/alex/Projects/atom/atom/script/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild"
gyp ERR! cwd /home/alex/Projects/atom/atom/script/node_modules/leveldown
gyp ERR! node -v v10.1.0
gyp ERR! node-gyp -v v3.6.2
gyp ERR! not ok
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR<EMAIL_ADDRESS>install: `prebuild-install || node-gyp rebuild`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the<EMAIL_ADDRESS>install script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/alex/.npm/_logs/2018-05-21T12_59_34_041Z-debug.log
child_process.js:617
    throw err;
    ^

Error: Command failed: /home/alex/Projects/atom/atom/script/node_modules/.bin/npm --loglevel=error install
../deps/leveldb/leveldb-1.20/util/hash.cc: In function ‘uint32_t leveldb::Hash(const char*, size_t, uint32_t)’:
../deps/leveldb/leveldb-1.20/util/hash.cc:37:9: warning: this statement may fall through [-Wimplicit-fallthrough=]
       h += static_cast<unsigned char>(data[2]) << 16;
       ~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../deps/leveldb/leveldb-1.20/util/hash.cc:39:5: note: here
     case 2:
     ^~~~
../deps/leveldb/leveldb-1.20/util/hash.cc:40:9: warning: this statement may fall through [-Wimplicit-fallthrough=]
       h += static_cast<unsigned char>(data[1]) << 8;
       ~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../deps/leveldb/leveldb-1.20/util/hash.cc:42:5: note: here
     case 1:
     ^~~~
In file included from ../../nan/nan.h:192,
                 from ../src/batch.cc:3:
../../nan/nan_maybe_43_inl.h: In function ‘Nan::Maybe<bool> Nan::ForceSet(v8::Local<v8::Object>, v8::Local<v8::Value>, v8::Local<v8::Value>, v8::PropertyAttribute)’:
../../nan/nan_maybe_43_inl.h:112:15: error: ‘class v8::Object’ has no member named ‘ForceSet’
   return obj->ForceSet(isolate->GetCurrentContext(), key, value, attribs);
               ^~~~~~~~
In file included from ../src/batch.cc:3:
../../nan/nan.h: In function ‘v8::Local<v8::Value> Nan::MakeCallback(v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*)’:
../../nan/nan.h:835:60: warning: ‘v8::Local<v8::Value> node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*)’ is deprecated: Use MakeCallback(..., async_context) [-Wdeprecated-declarations]
         v8::Isolate::GetCurrent(), target, func, argc, argv);
                                                            ^
In file included from ../src/batch.cc:1:
/home/alex/.node-gyp/10.1.0/include/node/node.h:172:50: note: declared here
                 NODE_EXTERN v8::Local<v8::Value> MakeCallback(
                                                  ^~~~~~~~~~~~
/home/alex/.node-gyp/10.1.0/include/node/node.h:88:42: note: in definition of macro ‘NODE_DEPRECATED’
     __attribute__((deprecated(message))) declarator
                                          ^~~~~~~~~~
In file included from ../src/batch.cc:3:
../../nan/nan.h:835:60: warning: ‘v8::Local<v8::Value> node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*)’ is deprecated: Use MakeCallback(..., async_context) [-Wdeprecated-declarations]
         v8::Isolate::GetCurrent(), target, func, argc, argv);
                                                            ^
In file included from ../src/batch.cc:1:
/home/alex/.node-gyp/10.1.0/include/node/node.h:172:50: note: declared here
                 NODE_EXTERN v8::Local<v8::Value> MakeCallback(
                                                  ^~~~~~~~~~~~
/home/alex/.node-gyp/10.1.0/include/node/node.h:88:42: note: in definition of macro ‘NODE_DEPRECATED’
     __attribute__((deprecated(message))) declarator
                                          ^~~~~~~~~~
In file included from ../src/batch.cc:3:
../../nan/nan.h: In function ‘v8::Local<v8::Value> Nan::MakeCallback(v8::Local<v8::Object>, v8::Local<v8::String>, int, v8::Local<v8::Value>*)’:
../../nan/nan.h:850:62: warning: ‘v8::Local<v8::Value> node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, v8::Local<v8::String>, int, v8::Local<v8::Value>*)’ is deprecated: Use MakeCallback(..., async_context) [-Wdeprecated-declarations]
         v8::Isolate::GetCurrent(), target, symbol, argc, argv);
                                                              ^
In file included from ../src/batch.cc:1:
/home/alex/.node-gyp/10.1.0/include/node/node.h:165:50: note: declared here
                 NODE_EXTERN v8::Local<v8::Value> MakeCallback(
                                                  ^~~~~~~~~~~~
/home/alex/.node-gyp/10.1.0/include/node/node.h:88:42: note: in definition of macro ‘NODE_DEPRECATED’
     __attribute__((deprecated(message))) declarator
                                          ^~~~~~~~~~
In file included from ../src/batch.cc:3:
../../nan/nan.h:850:62: warning: ‘v8::Local<v8::Value> node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, v8::Local<v8::String>, int, v8::Local<v8::Value>*)’ is deprecated: Use MakeCallback(..., async_context) [-Wdeprecated-declarations]
         v8::Isolate::GetCurrent(), target, symbol, argc, argv);
                                                              ^
In file included from ../src/batch.cc:1:
/home/alex/.node-gyp/10.1.0/include/node/node.h:165:50: note: declared here
                 NODE_EXTERN v8::Local<v8::Value> MakeCallback(
                                                  ^~~~~~~~~~~~
/home/alex/.node-gyp/10.1.0/include/node/node.h:88:42: note: in definition of macro ‘NODE_DEPRECATED’
     __attribute__((deprecated(message))) declarator
                                          ^~~~~~~~~~
In file included from ../src/batch.cc:3:
../../nan/nan.h: In function ‘v8::Local<v8::Value> Nan::MakeCallback(v8::Local<v8::Object>, const char*, int, v8::Local<v8::Value>*)’:
../../nan/nan.h:865:62: warning: ‘v8::Local<v8::Value> node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, const char*, int, v8::Local<v8::Value>*)’ is deprecated: Use MakeCallback(..., async_context) [-Wdeprecated-declarations]
         v8::Isolate::GetCurrent(), target, method, argc, argv);
                                                              ^
In file included from ../src/batch.cc:1:
/home/alex/.node-gyp/10.1.0/include/node/node.h:158:50: note: declared here
                 NODE_EXTERN v8::Local<v8::Value> MakeCallback(
                                                  ^~~~~~~~~~~~
/home/alex/.node-gyp/10.1.0/include/node/node.h:88:42: note: in definition of macro ‘NODE_DEPRECATED’
     __attribute__((deprecated(message))) declarator
                                          ^~~~~~~~~~
In file included from ../src/batch.cc:3:
../../nan/nan.h:865:62: warning: ‘v8::Local<v8::Value> node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, const char*, int, v8::Local<v8::Value>*)’ is deprecated: Use MakeCallback(..., async_context) [-Wdeprecated-declarations]
         v8::Isolate::GetCurrent(), target, method, argc, argv);
                                                              ^
In file included from ../src/batch.cc:1:
/home/alex/.node-gyp/10.1.0/include/node/node.h:158:50: note: declared here
                 NODE_EXTERN v8::Local<v8::Value> MakeCallback(
                                                  ^~~~~~~~~~~~
/home/alex/.node-gyp/10.1.0/include/node/node.h:88:42: note: in definition of macro ‘NODE_DEPRECATED’
     __attribute__((deprecated(message))) declarator
                                          ^~~~~~~~~~
In file included from ../src/batch.cc:3:
../../nan/nan.h: In member function ‘v8::Local<v8::Value> Nan::Callback::Call_(v8::Isolate*, v8::Local<v8::Object>, int, v8::Local<v8::Value>*) const’:
../../nan/nan.h:1479:5: warning: ‘v8::Local<v8::Value> node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*)’ is deprecated: Use MakeCallback(..., async_context) [-Wdeprecated-declarations]
     ));
     ^
In file included from ../src/batch.cc:1:
/home/alex/.node-gyp/10.1.0/include/node/node.h:172:50: note: declared here
                 NODE_EXTERN v8::Local<v8::Value> MakeCallback(
                                                  ^~~~~~~~~~~~
/home/alex/.node-gyp/10.1.0/include/node/node.h:88:42: note: in definition of macro ‘NODE_DEPRECATED’
     __attribute__((deprecated(message))) declarator
                                          ^~~~~~~~~~
In file included from ../src/batch.cc:3:
../../nan/nan.h:1479:5: warning: ‘v8::Local<v8::Value> node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*)’ is deprecated: Use MakeCallback(..., async_context) [-Wdeprecated-declarations]
     ));
     ^
In file included from ../src/batch.cc:1:
/home/alex/.node-gyp/10.1.0/include/node/node.h:172:50: note: declared here
                 NODE_EXTERN v8::Local<v8::Value> MakeCallback(
                                                  ^~~~~~~~~~~~
/home/alex/.node-gyp/10.1.0/include/node/node.h:88:42: note: in definition of macro ‘NODE_DEPRECATED’
     __attribute__((deprecated(message))) declarator
                                          ^~~~~~~~~~
In file included from ../src/batch.cc:3:
../../nan/nan.h: In function ‘void Nan::AsyncQueueWorker(Nan::AsyncWorker*)’:
../../nan/nan.h:1732:62: warning: cast between incompatible function types from ‘void (*)(uv_work_t*)’ {aka ‘void (*)(uv_work_s*)’} to ‘uv_after_work_cb’ {aka ‘void (*)(uv_work_s*, int)’} [-Wcast-function-type]
     , reinterpret_cast<uv_after_work_cb>(AsyncExecuteComplete)
                                                              ^
In file included from ../src/batch.cc:5:
../src/database.h: At global scope:
../src/database.h:86:7: warning: unnecessary parentheses in declaration of ‘pendingCloseWorker’ [-Wparentheses]
   void(*pendingCloseWorker);
       ^
In file included from /home/alex/.node-gyp/10.1.0/include/node/node.h:63,
                 from ../src/batch.cc:1:
/home/alex/.node-gyp/10.1.0/include/node/v8.h: In instantiation of ‘void v8::PersistentBase<T>::SetWeak(P*, typename v8::WeakCallbackInfo<P>::Callback, v8::WeakCallbackType) [with P = node::ObjectWrap; T = v8::Object; typename v8::WeakCallbackInfo<P>::Callback = void (*)(const v8::WeakCallbackInfo<node::ObjectWrap>&)]’:
/home/alex/.node-gyp/10.1.0/include/node/node_object_wrap.h:85:78:   required from here
/home/alex/.node-gyp/10.1.0/include/node/v8.h:9227:16: warning: cast between incompatible function types from ‘v8::WeakCallbackInfo<node::ObjectWrap>::Callback’ {aka ‘void (*)(const v8::WeakCallbackInfo<node::ObjectWrap>&)’} to ‘Callback’ {aka ‘void (*)(const v8::WeakCallbackInfo<void>&)’} [-Wcast-function-type]
                reinterpret_cast<Callback>(callback), type);
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/alex/.node-gyp/10.1.0/include/node/v8.h: In instantiation of ‘void v8::PersistentBase<T>::SetWeak(P*, typename v8::WeakCallbackInfo<P>::Callback, v8::WeakCallbackType) [with P = Nan::ObjectWrap; T = v8::Object; typename v8::WeakCallbackInfo<P>::Callback = void (*)(const v8::WeakCallbackInfo<Nan::ObjectWrap>&)]’:
../../nan/nan_object_wrap.h:66:61:   required from here
/home/alex/.node-gyp/10.1.0/include/node/v8.h:9227:16: warning: cast between incompatible function types from ‘v8::WeakCallbackInfo<Nan::ObjectWrap>::Callback’ {aka ‘void (*)(const v8::WeakCallbackInfo<Nan::ObjectWrap>&)’} to ‘Callback’ {aka ‘void (*)(const v8::WeakCallbackInfo<void>&)’} [-Wcast-function-type]
make: *** [leveldown.target.mk:113: Release/obj.target/leveldown/src/batch.o] Error 1
gyp ERR! build error
gyp ERR! stack Error: `make` failed with exit code: 2
gyp ERR! stack     at ChildProcess.onExit (/home/alex/Projects/atom/atom/script/node_modules/npm/node_modules/node-gyp/lib/build.js:258:23)
gyp ERR! stack     at ChildProcess.emit (events.js:182:13)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:235:12)
gyp ERR! System Linux 4.16.9-1-ARCH
gyp ERR! command "/usr/bin/node" "/home/alex/Projects/atom/atom/script/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild"
gyp ERR! cwd /home/alex/Projects/atom/atom/script/node_modules/leveldown
gyp ERR! node -v v10.1.0
gyp ERR! node-gyp -v v3.6.2
gyp ERR! not ok
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR<EMAIL_ADDRESS>install: `prebuild-install || node-gyp rebuild`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the<EMAIL_ADDRESS>install script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/alex/.npm/_logs/2018-05-21T12_59_34_041Z-debug.log

    at checkExecSyncError (child_process.js:596:11)
    at Object.execFileSync (child_process.js:614:13)
    at module.exports (/home/alex/Projects/atom/atom/script/lib/install-script-dependencies.js:9:16)
    at Object.<anonymous> (/home/alex/Projects/atom/atom/script/bootstrap:28:1)
    at Module._compile (internal/modules/cjs/loader.js:678:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:689:10)
    at Module.load (internal/modules/cjs/loader.js:589:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:528:12)
    at Function.Module._load (internal/modules/cjs/loader.js:520:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:719:10)

2018-05-21T12_59_34_041Z-debug.log

That's the same as #17370

@karthanistyr oh, it's so detailed, but without logs, as I can see… OK, I'm closing this issue, thank you.

Thanks for taking the time to contribute!
We noticed that this is a duplicate of https://github.com/atom/electron-link/issues/11. You may want to subscribe there for updates.
Because we treat our issues list as the Atom team's backlog, we close duplicates to focus our work and not have to touch the same chunk of code for the same reason multiple times. This is also why we may mark something as duplicate that isn't an exact duplicate but is closely related.
For information on how to use GitHub's search feature to find out if something is a duplicate before filing, see the How Can I Contribute? section of the Atom CONTRIBUTING guide.


Performance regression in negative binomial from 0.12 to 0.13 and onwards (at least for DeepAR in PyTorch)

Description
When loading the FOOD_3 subset of the M5 competition (cut to only data in 2016), I noticed that the performance of the negative binomial distribution changes, at least in DeepAR.
I suspect that something with the scaling is broken, but I haven't been able to pin it down unfortunately. When I look at the comparision between the two versions I can't really tell whether the negative binomial distribution changed.
Any help here is appreciated. The problem continues up to the current version.
To Reproduce
The example isn't really minimal, it's based on a notebook of the M5 data set available in colab (BUT IT DOESN'T RUN IN COLAB because of a lightning error with colab; locally it runs):
https://drive.google.com/file/d/1OOv_I7aAStgHW5iFuuKKB5r0qUW8BLxo/view?usp=sharing
Error message or code output
(Paste the complete error message, including stack trace, or the undesired output that the above snippet produces.)
The output shown here is after 15 epochs of training DeepAR on bespoke data where I've aggregated all forecasts and plot them against aggregated actuals. In v0.12 this produced an ok result after 15 epochs (it improves considerably with more epochs):

wheras in v0.13 this produces

Note however that there are small differences. The number of parameters is 76.3 K in v0.13 and in v0.12
The dataset that I'm loading is a pickled dataset which includes dynamic features (but I think I'm ignoring them in both v0.12 and v0.13)
Environment

Operating system: Mac OSX Monterey, ARM chip
Python version: 3.9.9
GluonTS version: 0.12 vs 0.13 and upwards
MXNet version: NA (this is for PyTorch, v 1.13.1)

@kashif @lostella I mentioned this to you some time ago and @jgasthaus FYI

@timoschowski inspecting the diff, one thing that changed is the dependency on PyTorch Lightning from 1.5 to >= 1.5. It seems like 1.7 introduced the MPS backend https://lightning.ai/pages/community/lightning-releases/pytorch-lightning-1-7-release/ which is one thing that might be causing trouble.
What version of lightning do you use?
Two options to check if this MPS thing is to be blamed:

pin lightning to 1.5 and see if it works better
set trainer_kwargs = dict(accelerator=“cpu”) when constructing the estimator, see if it’s better

I don’t see other changes between the versions that could explain this.

thanks @lostella, you're a wizard.
I have
import pytorch_lightning as pl
pl.__version__
'1.9.5'

when I do
"accelerator": "cpu"
the resulting output is still this:

however, when running the notebook with
!pip install -U "gluonts[torch]==0.13.0" matplotlib orjson tensorboard optuna datasets "pytorch-lightning==1.5"
results are like this for neg binomial, so indeed improved:

and performance is inline also after more epochs (500 here for v0.13 with lightning 1.5)

compare with (500 here for v0.12 with lightning 1.5)

For the moment I have a workaround by pinning the lightning version, so that's great. Huge thanks.
A couple of interesting things remain:

One thing that stands out for me is that all the distribution code shifted around, and imports are different. Did we change anything with the neg binomial implementation? Performance with student_t is exactly the same between v0.12 and v0.13 independent of lightning, so I find that curious. It doesn't really show up on the diff, so I'm wondering if you had any intuition here (I remember discussions with @kashif about this in the past)
why doesn't the notebook work on collab? Seems like the model loading doesn't work.

Of course the overall performance isn't there yet (eg peaks aren't aligned), but this is because I don't have any dynamic features included, will bring that back next.

adding some thoughts here. After a suggestion by @kashif I also tried running the notebook with
!pip install -U --pre torch torchvision torchaudio --index-url https://download.pytorch.org/whl/nightly/cpu
which gives me torch version:
'2.3.0.dev20240219'
however results are the same.

one thing I noted is that changing     context_length from the default prediction_length to 2*prediction_length has a substantial benefit here....

for v0.14 of GluonTS, a lighting version larger than 1.5 is required, so I'm stuck on v0.13... any idea here?

No, this is an issue; we'll have to figure out what's wrong with recent lightning versions and make sure that everything runs smoothly. Also, given that setting accelerator="cpu" did not work makes me think this may not be a problem on Apple silicon only? Running the same on Linux with recent versions of lightning would answer that

Did we change anything with the neg binomial implementation?

I don't think so: this is the history of changes, and @kashif's change is the only thing that happened. It's #2749 as was part of 0.13.0 already. It really seems like something weird is going on with training.

Ok, I have troubles running the notebook in colab. @kashif is this something that you could take a look at? this is about loading the models, something seems to be broken there....