-
-
Notifications
You must be signed in to change notification settings - Fork 376
docs: Resolve Docs build warnings #1427
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1427 +/- ##
=======================================
Coverage 88.47% 88.47%
=======================================
Files 94 94
Lines 18692 18692
=======================================
Hits 16537 16537
Misses 2155 2155 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
The patching likely happens after this warning. I don't think there's much we can do about it, other than checking to confirm that the patching is done correctly for all the warnings. |
I was really puzzled by these warnings. If you check these pages you will see they have broken links:
I'm not sure the first one ever worked, as it should have been So it seems these warnings are not just noise but real doc bugs. Side note, I forgot that nbsite has this feature that allows notebooks to be numbered ( |
|
@maximlt, these warnings could be fixed in a newer nbsite release. I have set over a dev deployment so that we can check it. Edit: Seems like they are still there. |
|
Yep I'm not sure how they could have been fixed by nbsite. At least for the two I checked, the links are clearly wrong and won't work in a Notebook or on the website. |
The ones you checked are different from the |
|
Please fix the changelog before you ask for review. Have you fixed the links @maximlt mentioned? |
|
Changelog file fixed. As for the other |
|
I think the fix links work; I just reran the dev webpage from this branch, and it looks good to me. Am I missing something? I'm pretty sure this is what does the conversion of links: https://github.com/holoviz-dev/nbsite/blob/645a25a3b2e05721c08f9d8b2a2fbe0455761f61/nbsite/scripts/_fix_links.py#L110-L110 |
|
There was a problem in the nbsite, which I have fixed in holoviz-dev/nbsite#344 |
hoxbro
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.
Some small comments. We shouldn't add the tiles notebook. If you want to add it, you can polish it and open a new PR with it.
pixi.toml
Outdated
|
|
||
| [feature.doc.tasks.docs-build] | ||
| depends-on = ['_docs-generate-rst', '_docs-generate'] | ||
| depends-on = ['_docs-install', '_docs-generate-rst', '_docs-generate'] |
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.
| depends-on = ['_docs-install', '_docs-generate-rst', '_docs-generate'] | |
| depends-on = ['_docs-generate-rst', '_docs-generate'] | |
| [feature.doc.tasks.docs-build-full] | |
| depends-on = ['_docs-install', 'download-data', '_docs-generate-rst', '_docs-generate'] |
I would still want it to be able to run the docs without installing.
This reverts commit fcfe5b5.
|
Some Deprecation warning is causing the tests to fail.. Edit: I think it was due to a new Pillow release. See python-pillow/Pillow#9018 |
datashader/tiles.py
Outdated
|
|
||
| # flip since y tiles go down (Google map tiles | ||
| img = fromarray(np.flip(arr.data, 0), 'RGBA') | ||
| img = fromarray(np.flip(arr.data, 0)).convert('RGBA') |
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.
Does this work for older versions of Pillow? Or should we version check it?
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.
Have done some changes, lets see how the CI likes it.
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.
I did not like it; I will revert the changes and address this in another PR.
|
Thank you for the work here 🔥 What was the final tally of warnings? |







closes #1421
This PR attempts to resolve most of the warnings and errors from the docs build, as well as some of the issues still pending in #1395