-
-
Notifications
You must be signed in to change notification settings - Fork 83
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
Supporting glob-style operations #123
Conversation
Thanks for submitting your first pull request! You are awesome! 🤗 |
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.
This LGTM!
I have a gut feeling that the larger ignore logic where this is a part could be refactored and reduce in complexity, but I think that is out of scope for this PR and especially not something to address now if I can't propose something concrete.
Co-authored-by: Erik Sundell <[email protected]>
I'm not currently using autosummary, so it's not easy for me to test that use-case. However, I have had an issue with sphinx-autobuild ignores in general, which may or may not be related. In the SymPy docs, we have a lot of plots that are generated by the matplotlib directive. Sometimes,
It does this one at a time for every image in the build, for which we have over 100. This happens even if I do If you want to reproduce this in the SymPy docs, you can install the dependencies from
then, after it is finished the initial build, in another terminal run
(the docs take a couple of minutes to build from scratch) The infinite loop honestly wouldn't be so bad, except the webserver gets blocked by it, meaning if you try to click a link in the live webpage, it tries to wait for the build to finish before it loads the page, and thus never loads. Making the webserver not block like this in general would be helpful. Happy to open a separate issue for this if you'd like. |
This sort of looping also happens whenever I checkout a new branch, one at a time for every file in the source repo that was changed. So I guess there are three problems here:
|
Hmm - I'm not sure why your build folder isn't being ignored ..indeed I would have expected this PR to fix it. I can try to take another look and see if i may have missed something. For the async and batch blocking stuff unfortunately i have no idea how that stuff works, and also have no real knowledge of this codebase other than what I learned while making this PR, so i think we should just open an issue to track it over time. (We also need to find a way to make a release, this repository got moved into executable books without transferring the ability to upload to pypi) FWIW I'd be more than happy to give you merge rights if you wanted to give a shot at fixing any of this! This repository basically has no maintainers right now |
Another quick thought: can you double check that the path to your I opened two issues to track two of your requests above:
For the "
I then set I then used this invocation of sphinx-autobuild:
and it correctly ignored the build folder. So I'm not sure why you aren't getting that behavior. |
Yes, I never actually committed
|
I spent a while trying to get the sympy docs to build this morning but could never figure it out. I got a string of error messages that I had to resolve by installing certain system libraries etc, but the last one I ran into I didn't know how to debug I am not sure why this doesn't work for you, but I think this PR at least solves some of the edge-cases that have been mentioned. I can remove the "closes" for your glob issue in case that is not resolved, but I'd suggest that we merge this one as an iterative improvement so that we can get it in before I go on paternity leave in a few days. |
I've seen that error before. I'll have to see if I can remember what causes it. |
And anyway, feel free to just open an issue for my problem. It's already a problem for me and this PR didn't seem to make it worse. Although I admit I didn't actually test the actual globbing nature of this PR. |
If it's not a problem could you open a SymPy issue with the problem you are seeing? |
What are the chances this getting approved? This is much needed! :) |
Sorry - I had a baby and dropped off the face of the earth. I can try to get back to this at some point but am on paternity leave for the time being |
Congrats!!! |
By the way, the repeated rebuilding issue doesn't just happen on SymPy. It seems to happen on any repo that uses autodoc with lots of Python files. Here's a simpler repo where I've also seen the issue that you can test https://github.com/quansight-labs/ndindex. |
@choldgraf Can you give this a rebase? |
@choldgraf Thank you for the PR. It would be fantastic if it could be merged. |
Hey all - I don't have time to work on this. I recommend somebody else picks up this PR. Feel free to fork and open a new one, or just take over this one |
Thank you for the answer. |
Done! For anybody else that comes across this PR, please check out its successor here: Thanks @jhgoebbert ! |
OK this one has bugged me for long enough that I decided to try and actually implement a fix.
This PR adds support for
glob
-module-style pattern matching, so you should be able to do--ignore **/*ignore*.txt
or--ignore **/ignore
and it should just work.I added a test that I think checks the edge-cases here, but welcome to have thoughts on how to improve.
--ignore
withdoc/_build/jupyter_execute/*.ipynb
to prevent infinite loop #117maybe @asmeurer or @consideRatio would be willing to give a look? I think both of you have gotten bitten by this one before as well :-)
Note that I also ran
pre-commit autoupdate
and ran it on all our files, because I think an outdated version was causing pre-commit to fail.