Thank you for your contribution@AryanSheka! Please put some information in the PR summary to tell us about the problem you are solving, with links to the issue being addressed and to the PR this replaces.

You will also need torebase your branch to fix a conflict in a file that has changed both here and on ourmain branch.

If you need help, feel free to ask questions here. Or you may prefer to ask them in ourincubator gitter room.

Hi,
The pr is related to#25796

I'm sorry I'm very new to this so I'm very confused about everything.

Awesome, thanks for pulling this together! No need to apologize, we'd be happy to help you out w/ whatever you're confused about if you tell us what's tripping you up. We also have a new contributors meeting next tuesday that may be of helphttps://scientific-python.org/calendars/

I have tried to fix the code as much as possible, but still some tests are failing and I do not know how to fix it. Can someone please
help me??

So the two tests that are failing are the xkcd image comparison tests, which means that this PR is probably messing with the default sketch. You can compare the expected and actual figures visually, see the image comparison test docs for how to find themhttps://matplotlib.org/devdocs/devel/testing.html#writing-an-image-comparison-test

So basically the image comparision test compares the expected image to the one we get from plotting. But if we see the main issue#13479 we are changing the sketch transformation of xkcd to make it give us a randomized pattern result rather than the repeating pattern result like the expected image.
So isn't it natural for the image comparision test to fail??

If I have understood the code comment, the pattern will be different than it was before, but it will still be deterministic because it just adds one to the previous seed value

So I think it should be enough to replace the baseline image with the result image that you now get?

Everytime I try to run the pytest in order to generate the images, I am always hit with the following error:

ImportError: cannot import name '_c_internal_utils' from partially initialized module 'matplotlib' (most likely due to a circular import) (/workspaces/matplotlib/lib/matplotlib/init.py)

I have tried fixing it but was unsuccessful.

Did you rebase since you installed Matplotlib? If so I think you will need to install it again as the C code will have to be re-compiled.

Well I have run into an issue, the expected image of xkcd and xkcd markers are:

But we are getting :

@AryanSheka are you sure what you're showing isn't the diff (expected-actual) image?

I'm sorry my bad, I thought they were the same thing, I have updated the baseline images and am still getting an image_comparision error.
Here are the images I recieved when I ran the tests after updating the baseline_images.

xkcd-expected

xkcd-recieved

xkcd-failed-diff

It doesnt matter how many times I replace the baseline_images, the new one produced is always slightly (not very noticable) different from the old one and the test fails. Even if you reinstall matplotlib after replacing the images, and then test it , they fail.

@AryanSheka can you do me a favor and just try to generate the image twice not as part of the test suite and check if they're the same?

@AryanSheka can you do me a favor and just try to generate the image twice not as part of the test suite and check if they're the same?

Yep here are 2 images generated together with the exact same parameters.

An online image comparision shows a small difference in the marked area even though the parameters were same.

@AryanSheka well done, I think now someone only needs to figure out where to allocateSketch::previous_seed (currently inpath_converters.cpp) and how to integrate that into the build process in a non-hacky way.

I have tried searching for other ways to allocate value to Sketch::previous_seed from the past few days, I could not find any other way to do this other than the way it is done in the code ( ie create path_converters.cpp and allocate value to Sketch::previous_seed in it). At this point I do not know what to do further.

I could not find any other way to do this other than the way it is done in the code ( ie create path_converters.cpp and allocate value to Sketch::previous_seed in it). At this point I do not know what to do further.

I tried and also did not succeed, it needs to be allocated both in_backend_agg and_path. So as you have it, it is probably the best we can do.

I think the biggest thing is in trying to implement my specific suggestions (which j appreciate the effort) it's losing sight of the big picture "what do you need to test to make sure this is implemented correctly?"

Well I have added tests, I will tell the significance of each of them:

This tests whether passing a seed to the Artist.set_sketch actually changes the global rcParam

This tests the same but for xkcd

This tests xkcd with default parameters (seed==0). This indirectly also tests whether passing a seed to xkcd impacts the sketch.

This tests whether setting a seed through artist impacts the sketch (it should)

This tests whether changing the rcParam impacts the sketch (it should).

Overall these tests cover all the changes made in this pr(including seed rolling) and can ensure that future commits do not break the changes the made in this pr.

Note: The image comparision tests also test the rolling charecteristic of seed

The relevant failures here are

FAILED lib/matplotlib/tests/test_path.py::test_xkcd_artist[png] - matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 20.469):FAILED lib/matplotlib/tests/test_path.py::test_xkcd_seed[png] - matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 20.341):

Corresponding diffs are:

Apologies for the delay in making the changes. Have been quite busy the past few week and also will be busy for some time in the future (possibly next few weeks), hence I wont be able to work on this pr during that time. In the mean time anyone can feel free to finish this.
If it is still open later, I will work on it.

Thanks for all the work so far, I rebased it to make it a little bit easier to review.

Hey@AryanSheka, just wanted to check about your availability for finishing this? otherwise I'll write finish this out.

Hey@story645, I don't think so I will be able to contribute to this pr further. Please feel free to move ahead.