No more Tuple, List, Dict#38797
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
|
The test failed with long output, that's also why I split the PR. I suggest splitting this into smaller ones. |
|
cc @ydshieh this is actually useful, though - we have a PR that always triggers the CI bug! |
this is not necessary due to long output, this happens from time to time, even with
Sorry, I d don't understand this command. |
There was a problem hiding this comment.
Thank you, I just randomly check 2 or 3 files and LGTM.
Please add @cyyever as co-author when click the merge button 🙏 Thank @cyyever to to make the library in a better shape!
The review page is somehow hanging due to large number of files.
The Too long with no output is known CircleCI issue, you can re-run the workflow.
If it persists, ping me and let's see what we could do
For the conflict, I guess it's easier to just reset the branch, fast-forward merge the latest main, apply the same changes and merge immediately once CI is green.
9fedad4 to
a3f2528
Compare
Hi @ydshieh, sorry, I should have been clearer. What I meant was that tests always fail on this PR with the |
|
Ok got it. But there is also some cases where this happens with just few outputs. But for those, rerins usually works |
|
this seems persist 😭 . @Rocketknight1 I can take a look to see if there is some hacky workaround, but if you are up to split the PRs, go ahead too. |
|
I don't think this is urgent! I can wait, but if you don't think it's worth fixing in the CI let me know and I'll make split PRs instead |
|
Is it due to short timeout values? The check took more than 10 minutes on my host. |
|
It's not something from our side but CircleCI. I can probably change the configuration there but I prefer not to. Ah, I might have some idea, let me try. |
|
I was able to make (I will try to see if I can make it available in a better way later) The 2 failing tests seems valid, so @Rocketknight1 could you take a look for them? When you want to commit the changes, use something like
Before merge, ping me for the revert (of some changes I made) |
|
@ydshieh nice, thank you! I'll get the CI to pass here and then ping you again. |
|
Just in case you need it, the change I made is to change from doctest_list = get_doctest_files()
print(f"\n### DOCTEST TO RUN ###\n{_print_list(doctest_list)}")to # doctest_list = get_doctest_files()
doctest_list = []
print(f"\n### DOCTEST TO RUN ###\n{_print_list(doctest_list)}")(and when push changes, use the commit message I mentioned) |
8fd8aa2 to
8a9ade4
Compare
|
cc @ydshieh tests are passing! Should be ready to revert + merge now |
|
Thanks @Rocketknight1 Thank you @cyyever again for the initiative, and apologize that we forgot to add you as a co-author in the merge commit 🙏 . |
This PR uses @cyyever's work to update types from Tuple/List/Dict to tuple/list/dict. We also add the UP006 ruff check to require this in future.