-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[MNT] testing code in README for correctness #676
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
base: main
Are you sure you want to change the base?
Conversation
fkiraly
left a comment
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.
Not sure what this is doing except adding >>> in front of code?
I do not see why we should do that.
Plus, you seem to be reformatting code so some lines do not fit on the screen anymore.
Maybe also let's not do this?
Plus my usual comments:
- please write a descriptive summary of your PR, I have a hard time understanding what you want to do here. Use AI if you need to, some description is better than none.
- where possible, do different things in separate PR, e.g., adding output coercion to
portfolio_performanceis not "test readme"
|
No, try to provoke the test with faulty code in README.md |
|
?? I am sorry, I genuinely have no idea what you are referring to. Can you please add a description to the PR and explain its purpose? There should be no |
|
Please ignore for now. Please read up on doctest |
|
@tschm, I know how doctest works and have written doctest plugins for pytest myself. Please don't assume lack of knowledge but provide an explanation or description what you are intending to do here. Regarding doctest: the |
|
I am not 100% sure on those >>>. I use them to indicate a doctest executable python command where as rows without them indicate the expected output (tested sign for sign). Yes, they make copy some inconvenient, but better than faulty code in a README... |
The entire point of the readme is to provide the most conveniently possible entry point to potential users. So I would strongly oppose any change that reduces user convenience substantially for a little bit of developer convenience. Speaking about developer convenience, it is easy to get the python blocks. Just grep anything between triple-backtick-python and triple-backtick lines. |
|
do I understand correctly, you want to test code in the README for correctness? (that it runs) That would be a very useful CI step! Though I would do it differently, and that avoids doing the business with the
Done |
|
Yes, I want to test the code in the README. So the >>> have an advantage >>> 2+2
4A few comments about this block. The "4" is the expected result. Replacing "4" with "5" should break the test. |
No description provided.