Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default inputs on steps in pipelines #850

Closed
ndw opened this issue Jul 23, 2019 · 4 comments

Comments

@ndw
Copy link
Contributor

commented Jul 23, 2019

I was surprised to discover 16.2.1. I don't remember agreeing that default inputs for ports should work that way, but I assume I was there when we decided it! :-)

So this is valid:

<p:declare-step xmlns:p="http://www.w3.org/ns/xproc" xmlns:test="http://test" version="3.0">
  <p:output port="result"/>

  <p:declare-step type="test:step">
    <p:output port="result"/>
    <p:input port="source">
      <doc/>
    </p:input>
    <p:identity/>
  </p:declare-step>

  <test:step/>
</p:declare-step>

Note that sequences aren't allowed on source, so at the graph level, this pipeline must fail. But we're saying that the default applies in this case, so even though there's no input to that port, that port still gets an input from its default.

Is this valid?

<p:declare-step xmlns:p="http://www.w3.org/ns/xproc" xmlns:test="http://test" version="3.0">
  <p:output port="result"/>

  <p:declare-step type="test:step">
    <p:output port="result"/>
    <p:input port="source">
      <doc/>
    </p:input>
    <p:identity/>
  </p:declare-step>

  <test:step>
    <p:with-input>
      <p:empty/>
    </p:with-input>
  </test:step>
</p:declare-step>

I assume it isn't, even though it's exactly the same at run time. In this case, the processor is supposed to detect that a sequence (the exact same sequence as the preceding case!) was provided on the source port and raise an error.

It seems more consistent to me to say that either both of these is an error or neither of them is.

On the one hand, I think this is subtle and prone to error. On the other hand, I've never seen a pipeline with default inputs outside of the test suite, so I suppose it won't be an error very often in practice.

It's pretty unpleasant to implement at the graph level given that the two graphs are identical and the data flowing through the graph is identical, but in one case it's not an error and in the other case it is.

@xml-project

This comment has been minimized.

Copy link
Contributor

commented Jul 23, 2019

Hhm, it seems to depend on how the graph is constructed: My implementation delivers <doc /> for the first pipeline, but XD0006 for the second.

I think these are the expected results (because it depends on whether there is a p:with-input). However I do not understand why we talk about a static error in the third and second case of
16.2.1: I think it can not be a static error because it depends on the inputs. So it has to be static. Additionally it is not an error, if sequence='true' is set for the port.
Did I misunderstood something?

@ndw

This comment has been minimized.

Copy link
Contributor Author

commented Jul 23, 2019

I think your results are consistent with what the spec says. I was whining about the semantics in the spec, which I don't like.

It's a static error because it does not depend on the documents in the pipeline. If you have any form of connection to the step, then the default inputs aren't used. If you have no connection to the spec and no default readable port, then the defaults in the step are supposed to be used. I think that's ugly and confusing, but it's what we spec'd.

I agree the second example wouldn't be an error if sequence="true" was specified. But that's a different issue. (My problem is that today my graph has no way of distinguishing between the first and second cases, a circumstance that I will have to fix.)

@xml-project

This comment has been minimized.

Copy link
Contributor

commented Jul 23, 2019

It's a static error because it does not depend on the documents in the pipeline.
Right. Thank you. I think we should fix the specs to say, which error is raised. Will create another issue for this, so we could close this.

@ndw

This comment has been minimized.

Copy link
Contributor Author

commented Jul 23, 2019

Yeah.

@ndw ndw closed this Jul 23, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.