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

[DOC] adding doctest guide to the testing documentation #4634

Merged
merged 14 commits into from May 27, 2023

Conversation

mdsaad2305
Copy link
Contributor

@mdsaad2305 mdsaad2305 commented May 21, 2023

Reference Issues/PRs

Fixes #4582

What does this implement/fix? Explain your changes.

Included doctest documentation within the testing and continuous integration section.

@mdsaad2305 mdsaad2305 requested a review from fkiraly as a code owner May 21, 2023 20:55
@mdsaad2305 mdsaad2305 changed the title Doctest [DOC] adding doctest to the documentation May 21, 2023
@mdsaad2305
Copy link
Contributor Author

Should I make any additional changes to the file to pass the code-quality section?

Copy link
Collaborator

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

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

Great, thanks!

Some small requests:

  • linting fails in this PR, please fix that
  • can you say explicitly how to run all doctests in sktime?

@mdsaad2305
Copy link
Contributor Author

mdsaad2305 commented May 22, 2023

Hey, just needed some help before I implement these changes.

  1. what kind of extension/package should we use to detect linting errors?
  2. When you mentioned running "all doctests," did you mean running doctest on all the files? I just want to make sure I understand that part correctly

@fkiraly
Copy link
Collaborator

fkiraly commented May 22, 2023

what kind of extension/package should we use to detect linting errors?

If you follow the link to "coding standards" on the page you are editing, I hope you'll find a guide that's explicit about this.
Let me know if it's not easy to find, then we may have to think about structure etc.

When you mentioned running "all doctests," did you mean running doctest on all the files? I just want to make sure I understand that part correctly

Yes, that's what I meant, thanks for asking.

@mdsaad2305
Copy link
Contributor Author

Could you let me know if the code for running all the files is correct? It seems to be working fine for me, but I hope I don't end up confusing the newcomers.

@mdsaad2305 mdsaad2305 requested a review from fkiraly May 22, 2023 15:08
@fkiraly
Copy link
Collaborator

fkiraly commented May 22, 2023

Hm, I would have done pytest --doctest-modules, does that not work?

@mdsaad2305
Copy link
Contributor Author

mdsaad2305 commented May 22, 2023

Might be an issue with my system but does using pytest normally take a long time?
Also, I think I'll use your command as it looks much simpler than mine and would be easier for newcomers to understand.
Thanks for the help

@mdsaad2305
Copy link
Contributor Author

is this alright?

@yarnabrina
Copy link
Collaborator

Hm, I would have done pytest --doctest-modules, does that not work?

doctest is a standard library, it's always present. And it runs only the doctests. pytest is not standard, and will be available only if someone installs dev dependencies. And it is going to run all unit tests along with doctests by default, so that immediately increases the time duration.

So, my suggestion is to include ways to run with both, and specify that one can try with the pytest version only if dev dependencies are installed. And may be they can use pytest to run just doctests in case they pass -k "not test_" or similar (not tested).

@fkiraly
Copy link
Collaborator

fkiraly commented May 23, 2023

So, my suggestion is to include ways to run with both, and specify that one can try with the pytest version only if dev dependencies are installed. And may be they can use pytest to run just doctests in case they pass -k "not test_" or similar (not tested).

Ah, I see - should we then mention both find . -name "*.py" -print0 | xargs -0 python -m doctest -v (on unix, no pytest installed) and pytest --doctest-modules (if you have pytest installed)?

@yarnabrina
Copy link
Collaborator

Exactly.

@mdsaad2305: can you also try to find a similar command for Windows users? It'll be good to document as well along with @fkiraly's above summary, if possible.

@mdsaad2305
Copy link
Contributor Author

Sure, I’ve some work till Friday, so I’ll try to finish it during the weekend

Copy link
Collaborator

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

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

Nice! Thanks, great compilation!

@fkiraly fkiraly added the documentation Documentation & tutorials label May 27, 2023
@fkiraly fkiraly merged commit 0099970 into sktime:main May 27, 2023
23 checks passed
@fkiraly fkiraly changed the title [DOC] adding doctest to the documentation [DOC] adding doctest guide to the testing documentation May 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Documentation & tutorials
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DOC] Include doctest section in developer guide
3 participants