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

Added select_one_from file and select_multiple_from_file, close #164 #171

Merged
merged 4 commits into from
Nov 6, 2019

Conversation

MartijnR
Copy link
Contributor

No description provided.

@MartijnR MartijnR requested a review from lognaturel July 19, 2019 17:52
@lognaturel
Copy link
Contributor

I feel like we should at least make some attempt of acknowledging that there's a connection between this and the external XML data section and contrasting it with the "Dynamic selects from pre-loaded data" and "External selects" sections. What do you think?

@MartijnR
Copy link
Contributor Author

Yes, and vice versa. Good point. Will try something and steer users to these 'proper XForm' solutions if they work for them.

@MartijnR
Copy link
Contributor Author

Forgot to commit this work. Let me know what you think (though no hurry)!

@lognaturel
Copy link
Contributor

Sorry about letting this sit. I have all kinds of thoughts. Would you be up for a 30min call to discuss? I think that might be more efficient than going back and forth as I have a number of questions for you about Enketo performance and external datasets in general. Let me know if so and I'll reach out to schedule.

@MartijnR
Copy link
Contributor Author

I have all kinds of thoughts.
Uh oh.... :D

Thanks, yes, I will ping you in the next few days to find a time. Now getting ready to battle a snow storm (and play in the snow).

@lognaturel
Copy link
Contributor

lognaturel commented Oct 24, 2019

Uh oh.... :D

I know! 😫

Now getting ready to battle a snow storm (and play in the snow).

Hard to imagine from here -- "we put the 'sweat' in 'sweater weather'" at highs in the 90Fs this week. Hope it all goes well and that there's some fun to be had! ❄️

@MartijnR
Copy link
Contributor Author

MartijnR commented Nov 1, 2019

I am quite curious now to try to load a form in Enketo with hundreds of thousand of data items that provides the rationale for select_external in ODK Collect. Do you perhaps have a real-life form for which select_external is required?

If it does load well (who knows, but probably not), it's worth exploring a transformation of select_external in enketo-transformer into regular external CSV data syntax (with a workaround for translations).

@lognaturel
Copy link
Contributor

nigeria_wards_internal_simplified.xml.zip is an example we discussed privately with ~10k items. This kind of form was the rationale for the features originally. It now performs reasonably well on modern devices and with performance improvements made over the last couple of years. Sadly, it doesn't look like there was any attempt to speed up the spec-compliant implementation before adding the database-backed implementations.

Unfortunately, I don't have access to real data that gets to larger scales. I'll see if I can get my hands on some or find a real deployment whose data we can mimic.

Copy link
Contributor

@lognaturel lognaturel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok with this. I expect there will be some confusion because it comes before the other options but it's probably not a big deal and we can adjust as needed.


| type | name | label | choice_filter |
| --------------------------------------- | ---- | ------------------------------ | --------------- |
| select_multiple_from_file country.csv | dest | Which countries did you live? | |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably be "In which countries did you live?" or "Which countries did you live in?". Does dest make sense as a name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Will change! Thx

| type | name | label | choice_filter |
| --------------------------------------- | ---- | ------------------------------ | --------------- |
| select_multiple_from_file country.csv | dest | Which countries did you live? | |
| select_one_from_file countries.xml | cou | Which country do you live now? | |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above -- "In which country do you live now?" or "Which country do you live in now?"

@MartijnR
Copy link
Contributor Author

MartijnR commented Nov 6, 2019

It now performs reasonably well on modern devices and with performance improvements made over the last couple of years.

Oh, that's great. I misunderstood you during our last meeting. So the path is open to promoting these new spec-compliant ways.

@MartijnR
Copy link
Contributor Author

MartijnR commented Nov 6, 2019

I'm ok with this. I expect there will be some confusion because it comes before the other options but it's probably not a big deal and we can adjust as needed.

Yes, I agree, external data is messy, reflecting implementations ;). Indeed, we can improve it later.

@MartijnR MartijnR merged commit 0d95bc2 into master Nov 6, 2019
@lognaturel
Copy link
Contributor

So the path is open to promoting these new spec-compliant ways.

I think so? Like you, I think I need to see examples of truly large datasets to see how they're performing these days.

I think another thing that's missing is documentation on the full workflow from "I have a CSV that was created by some other process" to "my form definition queries that data and uses it in selects".

It's unfortunate that columns named name and label are required for selects. We might look at making those configurable in XLSForm at some point.

For the actual querying, I go back and forth on how that's best done. As much as I don't love pulldata and find it personally confusing, perhaps it's the best option. I think we should have more resources and XLSForm support for more complex XPath but I don't know how reasonable it is for users to make deep use of (see this thread for a recent example).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants