I think we are already sorting all of those keys on the way out so do not need the ordered dicts.

Can you add an entry inhttps://github.com/matplotlib/matplotlib/tree/master/doc/users/whats_new and a test (like the deterministic svg test) that this actually works?

attn@jkseppan

I agree that a test is needed. I don't see the reason for the ordered dicts either, but there could be some subtlety that I'm missing. It would be neat to make the date settable by the user (via e.g. the PdfPages constructor) but of course not necessary for this particular use case.

I added a test for reproducible PDF output (you're right: this is very important!).
The test shows that the ordered dicts are needed for markers and hatches, due to the dict iterations at the beginning ofwriteHatches andwriteMarkers, which are unreproducible with python3.
Even if the same iteration occurs inwriteImages, the tests passes without ordering_images. I think this is because the_images keys are integers.
I also added two orderings (related to fonts), that appeared to be essential thanks to the test.

Oh, of course! I was thinking about the page stream, but it also matters in which order the objects are output at the top level of the pdf file. I suspect that the ordering is needed for_images too - maybe the dict implementation is a list for integer-keyed small dictionaries, or something.

I would like to add tests for postscript too, but I'm afraid the code will be nearly the same as for PDF: do you think I should move the common code somewhere (maybe in a newtesting/reproducible.py file), or duplicate it intest_backend_ps.py andtest_backend_pdf.py?

.SK...======================================================================FAIL: Test SOURCE_DATE_EPOCH support for PS output----------------------------------------------------------------------Traceback (most recent call last):  File "C:\Miniconda-x64\envs\test-environment\lib\site-packages\nose\case.py", line 197, in runTest    self.test(*self.arg)  File "c:\projects\matplotlib\lib\matplotlib\testing\decorators.py", line 152, in wrapped_callable    func(*args, **kwargs)  File "c:\projects\matplotlib\lib\matplotlib\tests\test_backend_ps.py", line 186, in test_source_date_epoch    _test_source_date_epoch("ps", b"%%CreationDate: Sat Jan  1 00:00:00 2000")  File "c:\projects\matplotlib\lib\matplotlib\testing\determinism.py", line 122, in _test_source_date_epoch    assert string in buffAssertionError--------------------------------------------------------------------

These failures on appveyor look real.

My naive guess is that there is a unicode vs bytes issues

These failures on appveyor look real.

Sure! I'm working on it but for now I can't reproduce the problem at home.

I think this is better now.

• For parallel tests, I think threads are used, and they share the same environment. So setting the SOURCE_DATE_EPOCH environment variable in a test is not good. So I used subprocesses instead.
• The timestamp was 'Jan 1' or 'Jan 01' sometimes, so I used Dec 15 instead to solve this issue.

Appveyor is now reporting only aHTTPError: 500 Server Error. I think this is not related to Matplotlib code, but I don't know if I can restart the build.

👍 from me, attn@jkseppan can you do a final review?

Commitc007f49 changes the tests to use a two-digit date in December instead of January 1st. I think the underlying problem should be fixed instead of avoiding it in the tests, since we don't want to tell users that theirSOURCE_DATE_EPOCH values must be two-digit days.

I considered reproducibility this way: with the same tools (same versions of all what you installed on your computer), the same commands lead to the same results.
With this point of view, I accept that with some different versions/implementations of python, the date can be written different ways (leading space or 0 for example). The problem with the tests was that we face different implementations with different behaviors. We don't pretend that the output will be exactly the same with all future versions of all the tools, all implementations of python.
I also accept that ghostscript can change its way to format the timestamp in a future version, but I did not include a test with usetex relying to the current behavior of ghostscript. Maybe this is too prudent.
Let me try to be more explicit about that in the doc.

I think the failure is due toghostscript (some versions now honorsSOURCE_DATE_EPOCH, but some don't).

The fail happens on the build where ghostscript is not installed, sobf7387e commit is not the right way to handle such situation.

The[XPASS(strict)] This test needs a ghostscript installation fail happened because the test should have failed (tagged@needs_ghostscript) but did not fail, becauseusetex was not properly passed to_determinism_save beforeaf4213e, so that the test ran without using ghostscript.
Correcting this, I then saw that now thatusetex is properly passed, the test usingusetex failed because ofghostscript, which does not honourSOURCE_DATE_EPOCH in the installed version. So I appliedbf7387e so that we can see if the test pass (goodghostscript version) or not, but without failing.

'power cycled to restart CI (appveyor failures looked due to qt-related packaging issues).

Other than one minor change, this looks good to go!

Don't worry about the coverall failure, that is un-related.

Sorry this has dragged on so long.

The coveralls drop is probably due to a change in master, and not this PR; a rebase to latest master might help that, but is not strictly necessary.

@QuLogic: thanks a lot for your review. I corrected those points.

A rebase might fix coveralls (as I'm pretty sure it was a change in master that dropped coverage), but it's not strictly needed. I guess we aren't really waiting on anything else here.

Thanks to you all for your kind support!

@JojoBoulix Thanks for taking care of this! Sorry it was such a protracted process.