-
Notifications
You must be signed in to change notification settings - Fork 136
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
Output no-repeat-reference-error
in some manner or the pyxform version or the xforms version
#393
Comments
Yes, I think we need to do this yesterday. I much prefer using a general version attribute. I'd be in favor of doing what we do on other ODK tools and using a commit hash to represent version. That way the versioning can be automated, is guaranteed to be unique, and those of us running off XPath Enthusiasts Unite. 🤘 |
Great! Automating would be ideal. The only issue with a hash is how to do a In Enketo - nodeJS - we do |
Would this be a good location , attribute name, namespace? <model>
<instance>
<data id="mysurvey" orx:version="2014083101" odk:pyxform-version=".....">
.....
</instance>
</model> |
Ah, indeed. We do Or perhaps |
Thanks! I believe I wonder how likely it is that an organization runs a fork (or deploys a branch) with a minor customization (though perhaps with lots of commits for it). The advantages of using tags is that that may be easier to deal with since the tag we're looking for has the feature. |
Fair enough. Probably clients would want to use something like a regex to pull the three digits because I realize I confused things initially by saying "using a commit hash to represent version" when I really meant
We of course don't know for sure but I'd guess it's not common. Experience suggests that it's pretty hard to maintain a fork up to date because of the structure of the code so either folks have wandered off entirely or they're on core. |
Process-wise, where do we go from here? I think the Does anyone specific need to comment on the nature of the version provided? Presumably @ukanga should sanity check at the very least. Can you do the implementation, @MartijnR? |
Yes, I agree.
I was going to say "no, I don't speak python", but now I feel embarrassed and challenged. I am going to try today! (and will add @ukanga, and you as reviewers - we can always change the version algorithm in the PR if somebody has an argument to do so). |
Explicitly version-tagging ODK XForms seems to make a lot of sense. However I'm not sure this should be against the particular version of pyxform (possibly?) used to generate it; wouldn't it make more sense to tag it with an appropriate ODK XForms spec version instead? [didnt we discuss versioning the ODK spec at some point? maybe now's a good time...]. For example, I often manually write certain XForms - how would I know what How about [BTW, I'm tempted to nominate |
Haha, I actually (truly) think that attribute is very self-explanatory, but am delighted that you added the word "abstruse" to my vocabulary, and you make a good point about versioning. This trigger issue for versioning demonstrates a small problem with linking to an XForms Spec version. The use case that triggers this versioning isn't directly linked to an XForms spec change. Using relative references to refer to nodes inside a repeat has always been allowed and supported. Enketo would just like to know if another deprecated (absolute path) usage is guaranteed to not be used in this form. However, I agree this solution is very pyxform-centric and that is not ideal. We could workaround the xforms-spec issue by removing the incorrect absolute-path usage now (at more or less the same time as we introduced the correct syntax in pyxform) and generate a new XForms spec version. Hmmm. |
I think @tiritea's solution is better as it is form-builder agnostic. I agree with the proposed attribute. The relative-paths in repeat refs are a bit of a special case. Generally, linking to an XForms version is probably a good solution to detect XForms changes that cannot easily be determined from analyzing the XForm content itself (which I think would normally be my preferred approach). If we name the current XForms version It is possible (and likely) that an XForms feature has no implementation in pyxform/xlsform, but I think that's okay. The client must not consider version |
👍
Agreed. I dont think a client necessarily has to claim |
I think the problem we're trying to solve is a form builder problem, not an XForms problem because form builders implement features at different rates. Perhaps if we wanted to be more generic we could do I think that it so happens that introducing I think we'll have a number of similar cases where form rendering clients or servers would like to know about the form builder version. The changes to make the meta block ODK XForms spec compliant are another potential example. I agree that it could be useful to also capture XForms spec compliance level but that feels different to me. I'm also not really sure what level |
But isnt the issue that pyxform is generating non-standard XForm XPath expressions - which happen to be customizations introduced by our ODK XForm spec? If so, then in effect these non-standard, problematic features are ultimately tied to our particular ODK xforms spec, and its associated version. Right? It just seems odd to me that an (ODK standard-compliant) XForm would have a version tag for a specific generation tool, which itself is should - ideally - advertise conformance to some particular version of said spec (!). So if we're not using the ODK spec version as the basis for the toolchain, then it seems like we may be missing something somewhere else... [sic] [additional note: spec conformance can have optional components, so its not like pyxform has to do 100% everything doc'd in the spec]
I'm confused... if a pyxform change doesn't actually change the resulting XForm/XPath, then who cares?! (note, I'm grouping specific XPath support with a particular XForm version. So changes in XPath support could/should trigger an odk:xforms-version change I would think) |
I'm starting to think we need both (and I like
I don't really see the repeat-ref issue as a form-builder problem. For me the initial proposed
Missing features wouldn't matter for pyxform the way I see it. If pyxform has currently no way to generate
I think the metadata placement issue could be seen from 2 different perspectives: (1) as a significant deviation with the spec or (2) a bug that was found after claiming to produce valid x.x.x ODK XForms output. In the former case, it would have to be fixed before we can add odk:xforms="1.0.0" to the output. In the latter case, it's too late to do so, and
I also remember we've discussed other candidates, but I cannot think of them at the moment. If you remember, it may be good to add them. |
So the ODK spec says:
So basically, in the current version of the ODK spec (0.9.0?), absolute XPath expressions are/will be interpreted as relative to the current repeat node. Whereas in some future version of the ODK spec (1.0.0?), absolute XPath expression will be interpreted relative to the actual instance root (/). So if I (or pyxform...) produce an XForm where I have written an absolute XPath expressions with the understanding that it should be interpreted relatively inside a repeat group, then I should tag my XForm as conforming to I guess I'm still not seeing why odk:xform-version doesn't convey exactly what we want to specify here?... A client knows, by looking at odk:xform-version, exactly how it is expected to handle the form's XPath expressions; and either it already does (because that's how the client's XPath processor is written), or it doesn't/cant (in which case it can reject the form as unsupported), or - ideally - it might be able to support both approaches and can use the version tag in order to [this leave unspecified what we do with forms lacking any version tag, but we face the same issue regardless... Probably assuming the lack of an explicit odk:xform-version implies a legacy form, ie 0.9.0 functionality...] |
I agree that this should be what we aim for. I think it is now. But (and I feel like I'm repeating myself across several conversations, sorry!), we can't erase 10+ years of history. There are numerous bugs and omissions in pyxform that we've redressed recently and probably more to go.
Apologies -- I meant to say that pyxform behavior may need to change without an XForms spec change. Another example of that would be the repeat output (#380). We've learned that at least one major client (Enketo), was "modifying" form definitions to get a particular behavior. It seems to be the behavior that users want. So we're now modifying the pyxform output. In that case, I think having the form generator information will be helpful for end user support more than anything. In fact, I should have said that earlier, because it's for support that I've wanted form builder version information in the past.
I see. I was thinking of it strictly as Enketo removing its pre-processing step but I see what you mean about broadly any client perhaps supporting either behavior. The deviation which came from the initial JavaRosa implementation was not quite correctly characterized. It is only if a single node type is requested. Instead of returning the first node as in standard XPath, it evaluates it in the context of the enclosing repeat. An absolute path in a context where a nodeset is requested works as expected. The problem is that this different behavior based on whether a nodeset or single node is requested is hard/impossible to get with an off the shelf XPath evaluator. So in Enketo (correct me if I'm wrong, @MartijnR) it has never been possible to use absolute XPath expressions that return nodesets in repeats.
All that to say that I'm personally more interested in knowing what tool generated a certain form. But y'all are right that in the case of the interpretation of absolute paths it's an XForms spec issue. If we're going to introduce
I agree with this in principle but I don't see how it works practically. It seems that when new features are added to the ODK XForms spec, that should be captured in some kind of version change, no? That would seem to require that form builders release support for features in the same order to be able to claim a particular spec version. Or perhaps the attribute could be
Will ponder. |
I agree that knowing what specific tool (+ version thereof) generated a form can be useful information, but that does rather defeat the purpose of having a 'specification' [sic]... 😉 But in all seriousness, making explicit what exact tooling was use further upstream in the tool-stack is a bit of a slippery slope, one that can lead (inadvertently?) to a more 'closed' ecosystem. I also understand the concern around introducing an odk:xforms-version - eg what to do when new spec features are introduced, what if certain features aren't supported by a particular client, etc - but I think there can be a lot of flexibility around what conformance actually means. For example, being "conformant to In truth, if we basically own the ODK spec, then we can define the sematics of what its versions mean. |
BTW, I hope I'm not appearing too obtuse [or should that be 'acute'...]. End-of-day I'll almost certainly ignore any |
Let me actually walk back my prior enthusiasm for This became a problem as tools using off-the-shelf XPath evaluators joined the ecosystem because they couldn't get the absolute-as-relative behavior just for single node evaluation. It's more common to have forms with relative references that refer to single nodes than to nodesets. So clients like Enketo sacrificed the latter for the former. What Enketo needs to know is "can I be confident that relative references in repeats used in a context where a single node is needed are represented as actual relative paths" so that it can correctly interpret absolute paths that refer to nodesets. If it can't have that confidence, it would rather err on the side of interpreting absolute paths in repeats as relative because that's the more likely case. That doesn't have anything to do with ODK XForms spec level, it's purely about the form definition.
I think what you mean is that if clients of form definitions start having special cases for different form builders, it makes it harder for new/smaller tools to get recognized, right? I see what you mean. To me, this information is largely for troubleshooting and for letting clients of forms deal with significant changes in old form builders that have produced forms for a long time. I think we can specify it as such. I don't think it would be an issue for new tools because those new tools actually would comply to specifications or their bugs would likely be spotted and rectified quickly. The problem we have here is again,
As will Collect, Central, etc. But I think that's ok -- I expect it will be quite helpful for forum support. I do think the attribute name should be something more general about form source, though ( |
I go back and forth !
Strictly speaking the pyxform change a short while back was not spec-related indeed. We would be pretending it was by removing the deviation from the spec at the same time (which is now done) and generate a spec version. I think both ways of looking at the Enketo requirement prompting this discussion are valid. I don't mind viewing it as pyxform specific, but still find it a little easier to reason from an XForm spec perspective. And this is still a remaining sticking point: |
I suppose this could be seen as a hack/abuse as well, but I was thinking more that if a It's true that we can only do this once but I would find it really surprising if we found something of this magnitude that affected all form definitions made up to the present. If there is such a thing and we haven't discovered it yet, it seems it would have to be fairly niche. |
That makes me wonder if the builder attribute value should be a username and Enketo can just look for But, yeah, checking for the presence of this attribute and not the value seems quite a hack, though I guess it would work. |
Syntactically, how does that really differ from assuming anything with odk:xforms-version >= 1.0.0 ? |
BTW I think you should probably add "martijnr" to your rogues gallery... :-) |
It doesn't differ syntactically but it feels more true to the problem that we're trying to solve here. More broadly, I'm not convinced that the |
I'm now okay with an Any evolved opinions on your side @tiritea? |
Should this |
Yes, it should (once fully agreed). |
The discussion continued on Slack and so far we've 'decided' to use |
no-repeat-reference-error
in some manner or the pyxform versionno-repeat-reference-error
in some manner or the pyxform version or the xforms version
* Recognize select questions from the question elements themselves instead of checking the bind type; see XLSForm/pyxform#168 * Recognize new error message when a group name matches the form name See XLSForm/pyxform#510 * Set `allow_choice_duplicates` to `yes` in cascade test XLSForms See XLSForm/pyxform#23; XLSForm/pyxform#373 (comment) * Update tests to use `default_name` instead of relying on the XLSForm file name See XLSForm/pyxform#130 * Update expected XML in tests to match new pyxform behavior: * `<model>` becomes `<model odk:xforms-version="1.0.0">` * See XLSForm/pyxform#393 * The bind type for `select` and `select1` becomes `string` * See XLSForm/pyxform#168 * `calculate="concat('uuid:', uuid())"` becomes `jr:preload="uid"` * See XLSForm/pyxform#94 This squashed set of changes includes work from #699. Thanks to @duvld for that contribution.
* Recognize select questions from the question elements themselves instead of checking the bind type; see XLSForm/pyxform#168 * Recognize new error message when a group name matches the form name See XLSForm/pyxform#510 * Set `allow_choice_duplicates` to `yes` in cascade test XLSForms See XLSForm/pyxform#23; XLSForm/pyxform#373 (comment) * Update tests to use `default_name` instead of relying on the XLSForm file name See XLSForm/pyxform#130 * Update expected XML in tests to match new pyxform behavior: * `<model>` becomes `<model odk:xforms-version="1.0.0">` * See XLSForm/pyxform#393 * The bind type for `select` and `select1` becomes `string` * See XLSForm/pyxform#168 * `calculate="concat('uuid:', uuid())"` becomes `jr:preload="uid"` * See XLSForm/pyxform#94 This squashed set of changes includes work from #699. Thanks to @duvld for that contribution.
For discussion.
Some advanced XPath enthusiasts (@ln) are running into issues caused by Enketo's workaround for a major XForm syntax issue: https://opendatakit.github.io/xforms-spec/#a-big-deviation-with-xforms.
Pyxform recently started generating relative repeat node references, so we're on the verge of being able to remove this nasty deviation from the ODK XForms spec (I think).
However, since we still need to be able to run forms that were transformed by older versions of pyxform, it will be many years before we can remove the code workarounds in Enketo and Collect. This means it is a challenge, for Enketo, to facilitate support for those very cool advanced forms.
This may be a good reason to add some versioning or featuring in the XForms output. E.g. the output could contain a
no-repeat-reference-error
class or apyxform-version
attribute. Data collection clients could disable the bug-causing workaround for the buggy XForms syntax based on the presence of this class or the value of the attribute.I imagine there will be more valid use cases in the future, so perhaps a version attribute is the easiest?
The text was updated successfully, but these errors were encountered: