TST: fix read_stata doctest #42670 #42701
Conversation
fangchenli
left a comment
fangchenli
left a comment
There was a problem hiding this comment.
We want them fixed rather than skipped.
fangchenli
changed the title
|
Yes but the only solution I can see is creating a dummy .dta file for it to work since it needs a file to be read and I don't know where you would store that. For example, should I create an examples folder to store it so all future example documentation code can run as well? |
|
Hello @KrishnaSai2020! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2021-07-31 07:38:51 UTC |
|
Made the requested changes @fangchenli. |
|
This is ruining the docstring's purpose just to pass a test. It is not a good idea. The entire point of reading with chunks to to parse a large data file. IT should have > 10,000 roww to be illustrative. |
|
@bashtage I see what you mean. I made the changes to the df's size please review the new changes. |
|
@bashtage thanks for you help. I have now made the changes. |
|
@bashtage I think I've fixed the formatting now. Please do let me know if there is anything else |
bashtage
left a comment
bashtage
left a comment
There was a problem hiding this comment.
Still a bit of formatting.
|
@bashtage I've added this doctest into the scripts and resolved the conflicts |
|
This should now pass all checks. |
|
From what I can tell everything works apart from the doctests in Style_render.py which is unrelated to this pr. |
|
@fangchenli Could I get an update as to what is going to happen with this pr, please? Is it going to get merged? or have I done something wrong which means it can't get merged? |
bashtage
left a comment
bashtage
left a comment
There was a problem hiding this comment.
Need to remove new module that doesn't pass, and small grammar change
|
@fangchenli please re-review |
|
Could I get a second opinion on this PR, please? it's passed all checks and already has one approval |
|
@KrishnaSai2020 This LGMT. I'll leave it to @jreback to make the final decision. You don't need to merge master in anymore unless there is a conflict. |
|
@KrishnaSai2020 Thanks. |
* doctest fix for pandas-dev#42670 * added read_stata docstring into the doctests Co-authored-by: KrishnaSai2020 <[email protected]>
4 participants
Uh oh!
There was an error while loading. Please reload this page.