Framed @graph @container output not properly compacted

[dlongley]

http://tinyurl.com/yaa98egk

Input

{
  "@context": {
    "@version": 1.1,
    "@vocab": "https://example.com#",
    "claim": {
      "@id": "ex:claim",
      "@container": "@graph"
    },
    "id": "@id"
  },
  "claim": {
    "id": "ex:1",
    "test": "foo"
  }
}

Frame

{
  "@context": {
    "@version": 1.1,
    "@vocab": "https://example.com#",
    "claim": {
      "@id": "ex:claim",
      "@container": "@graph"
    },
    "id": "@id"
  },
  "claim": {}
}

Output

{
  "@context": {
    "@version": 1.1,
    "@vocab": "https://example.com#",
    "claim": {
      "@id": "ex:claim",
      "@container": "@graph"
    },
    "id": "@id"
  },
  "@graph": [
    {
      "claim": {
        "@graph": {
          "id": "ex:1",
          "test": "foo"
        }
      }
    }
  ]
}

Expected Output:

{
  "@context": {
    "@version": 1.1,
    "@vocab": "https://example.com#",
    "claim": {
      "@id": "ex:claim",
      "@container": "@graph"
    },
    "id": "@id"
  },
  "@graph": [
    {
      "claim": {
        "id": "ex:1",
        "test": "foo"
      }
    }
  ]
}

[dlongley]

cc: @gkellogg, @davidlehn

[gkellogg]

On it; it seems to work okay if you frame it, expand it and then compact the results with the context in the frame, so there is some setup issue at the top of the frame algorithm that needs to be figured out.

Thus far, we haven't done many crossover tests, just focusing on expansion and compaction for these container bits, so we should probably add more such tests.

[gkellogg]

The issue seems to be the addition of a graph name in the intermediate expanded result:

[
  {
    "@id": "_:b0",
    "http://example.org/claim": [
      {
        "@id": "_:b1",
        "@graph": [
          {
            "@id": "http://example.org/1",
            "https://example.com#test": [
              {
                "@value": "foo"
              }
            ]
          }
        ]
      }
    ]
  }
]

Because there is a graph name, it is not considered a simple graph, and step 8.7.7.3 represents it using @graph. Without knowing if _:b1 is used anywhere else, we can't do anything else.

While the algorithm performs pruneBlankNodeIdentifiers, my implement actually does this when cleaning up @preserve, as I suspect your implementations do as well, which is an easy enough fix, but this would generate different results if that option were false. Alternatively, we could use the code to identify them, and pass the list to the compaction algorithm for precisely this reason, even if identifiers are not otherwise removed, but at increased complexity.

Thoughts?

[dlongley]

@gkellogg,

While the algorithm performs pruneBlankNodeIdentifiers, my implement actually does this when cleaning up @preserve, as I suspect your implementations do as well, which is an easy enough fix, but this would generate different results if that option were false.

At a minimum we should do this -- however, I think we should actually do your other recommendation:

Alternatively, we could use the code to identify them, and pass the list to the compaction algorithm for precisely this reason, even if identifiers are not otherwise removed, but at increased complexity.

It seems that this is really what needs to be done so that people aren't surprised by the results.

[gkellogg]

On reflection, passing a list to compaction, and on to the simple graph check is more complex, and just results in the prune blank nodes being run for just this case. Given that the blank node identifiers are useless if they could be pruned, maybe we should just not make it optional for version 1.1. Alternatively, we could always prune them where it would result in a simple graph, but this seems like needless complexity. In 1.0 they were just a byproduct of flattening and not really useful in and of themselves.

[dlongley]

I'm fine with always pruning. I think it was surprising to many people that they were added after framing to begin with.

[dlongley]

This appears to be fixed, closing.