-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Fix pickled doctrees cache. #11939
Fix pickled doctrees cache. #11939
Conversation
For now, I'll merge this since I don't see any other simple solution. It will increase the build time of all projects, yet it will not cause any issues with rebuilds. However, I assume that it should not be too stressing to read 70ks pickled doctrees on modern systems (I could have been concerned 30 years ago but I think the performance drop is marginal). |
Is there an ETA on the release for this? TIA |
Err... technically, it's fixed on master. However, I don't know when we'll be able to release 7.3 so.. I cannot give an ETA on the 7.3 release, sorry :( |
No worries. Do you think I could patch this in to 7.2.6 without any other issues? Seems simple enough... |
Mmh. I don't think it would affect other stuff in the code. At least it shouldn't. So technically, you could patch your local installation. However, I'd recommend just installing Sphinx 7.3 using |
Unfortunately it breaks one of my extensions (autodoc_pydantic), because it looks like it's trying to import Is this a deliberate API change? I can report it upstream or make a new issue here.
|
Ah, yes. I see that we really need to be on the same page on what is public API and what's not. So here's an example of something that we think as private API (non-capitalized module-level constant) however users as using it as if it were public (although not documented in the RST documentation). Most of those public/private API issues stem from 10y.o. code base where people had different conventions and where we did not expect things to be used if not explicitly documented. Personally, I don't expect users to use non-capitalized module-level constants as if they were public API. (long story short, the rationale behind the refactorization is that the Python domain was too big of a file so many things were moved into 'private' API because we did not expect them to be used directly by extensions but it appears we are breaking things, so maybe we should re-export them normally but deprecate them). I'm not sure you should report this upstream since it's our fault I would say. We did not clarify what is public/private so we should somewhat fix that before releasing 7.3 |
Not heard that one before 😅 |
Well.. technically, PEP 8 tells you that:
However, they also say that you should use underscored names. For constants, they recommend you to use capital letters. So I would assume that non-capitalized constants are not meant to be publicly used. Anyway, I think we should create a separate issue to make anything that could have introduced breaking changes and make them explicitly deprecated (and after that, we'll be good and won't have problems anymore I think). |
Closes #11474.
Closes #11908.
Note that this fix may increased build time for very large projects since we need to revert back to reading at least twice the pickled files.
Alternatively, we could add the possibility to mark which document should be 'refreshed' from its file if needed? though I don't know which ones we should actually target. Maybe @AA-Turner has some solution for this?