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
Example to demonstrate copy_header_from
in math_img
#4392
base: main
Are you sure you want to change the base?
Conversation
👋 @man-shu Thanks for creating a PR! Until this PR is ready for review, you can include the [WIP] tag in its title, or leave it as a github draft. Please make sure it is compliant with our contributing guidelines. In particular, be sure it checks the boxes listed below.
For new features:
For bug fixes:
We will review it as quick as possible, feel free to ping us with questions if needed. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4392 +/- ##
==========================================
+ Coverage 91.85% 91.94% +0.08%
==========================================
Files 144 143 -1
Lines 16419 16639 +220
Branches 3434 3529 +95
==========================================
+ Hits 15082 15299 +217
+ Misses 792 769 -23
- Partials 545 571 +26
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
The same MSDL dataset download fails during building the docs. So maybe first merging #4390 would be better. |
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.
The example LGTM.
I'm still not 100% sure that this deserves a full example, but I find it quite pedagogical, so I'm +0 for this addition.
Indeed, it has to come after #4390.
Yes, maybe it doesn't make much sense to have it for just one function. But as explained in #4393, I identified the same issue in several others under the |
Can you propose an enhanced example ? |
Yes, but I should fix #4393 first. |
So, all the other
|
We should decide on this at a forthcoming coredev meeting. |
math_img
#4362Changes proposed in this pull request: