-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Document how to test examples in user guide, add some more coverage #11178
Conversation
As a newcomer, I can say that more guidance on the examples is good! I think it might also be helpful:
I'll also note that the for newcomers the hardest part of examples - and tests - is figuring out how to setup the appropriate pre-conditions. Maybe we could create some "best practices" around how to approach this. Or perhaps its just a matter of reading enough existing examples and tests! |
Thanks @efredine, this is great feedback
Yes, I agree this would be helpful. I'll think if I can come up with some summary that matches current reality (rather than what I would ideally liek)
Yes, I agree -- I think this is the approach that @tshauck took with the user guide (e.g. https://datafusion.apache.org/library-user-guide/working-with-exprs.html has links to various examples)
When you say "preconditions" what do you mean? Is it the correct data directories / data files checked out? |
By pre-conditions I meant that in order to test or illustrate something like an option when reading a file I need to first create a file and write to it with data that is meaningful for the thing being tested. |
🤔 Maybe we could add some examples of "how to create test data" (specifically we can now use the |
// | ||
// For example, if `user_guide_expressions(line 123)` fails, | ||
// go to `docs/source/user-guide/expressions.md` to find the relevant problem. | ||
|
||
#[cfg(doctest)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it turns out we already have some coverage of tests in the user guide, but it was somewhat unclear. I documented how it works
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm thanks @alamb for documenting this part!
Thank you for your review (as always @comphead ) 🙏 |
…pache#11178) * Document testing examples, add some more coverage * add docs
Which issue does this PR close?
Part of #11172 and #1813
Rationale for this change
I am trying to make the examples easier to find / navigate. Right now there are many files in
datafusion-examples
but they are somewhat overwhelming. There are also some examples in the user guide, but many are not testedI tried to consolidate the examples in #11173 but @lewiszlw (rightly) pointed out that one giant .rs example file might be even harder to find / navigate
I think the ideal outcome from a users perspective is for the examples to be inline in the user guide so they are both discoverable and the context explained. However, it is critical that the examples be testable
What changes are included in this PR?
expressions.rs
and fix the example there.Are these changes tested?
Yes,
Are there any user-facing changes?
Documentation on tests.