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

Ensure frozen object descriptions are reproducible #5388

Merged
merged 1 commit into from Sep 5, 2018

Conversation

Projects
None yet
2 participants
@lamby
Contributor

lamby commented Sep 5, 2018

Whilst working on the Reproducible Builds effort, we noticed that sphinx could generate output that is not reproducible.

In particular, the rendering of frozenset objects in default arguments and elsewhere is currently non-determinstic.

For example:

frozenset(['a', 'b', 'c'])

Might be rendered as any of:

frozenset({'a', 'b', 'c'})
frozenset({'a', 'c', 'b'})
frozenset({'b', 'a', 'c'})
frozenset({'b', 'c', 'a'})
frozenset({'c', 'a', 'b'})
frozenset({'c', 'b', 'a'})

Patch attached that sorts the contents of frozensets whilst rendering. This is parallel to the dict and set type logic

@lamby lamby changed the title from Ensure frozetset object descriptions are reproducible to Ensure frozen object descriptions are reproducible Sep 5, 2018

@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Sep 5, 2018

Codecov Report

Merging #5388 into master will decrease coverage by <.01%.
The diff coverage is 78.94%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5388      +/-   ##
==========================================
- Coverage    82.5%   82.49%   -0.01%     
==========================================
  Files         286      286              
  Lines       39112    39131      +19     
  Branches     5932     5936       +4     
==========================================
+ Hits        32268    32283      +15     
- Misses       5431     5433       +2     
- Partials     1413     1415       +2
Impacted Files Coverage Δ
sphinx/util/inspect.py 55.2% <100%> (+0.67%) ⬆️
tests/test_util_inspect.py 80.89% <66.66%> (-0.67%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 342de4f...17d32d6. Read the comment docs.

codecov bot commented Sep 5, 2018

Codecov Report

Merging #5388 into master will decrease coverage by <.01%.
The diff coverage is 78.94%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5388      +/-   ##
==========================================
- Coverage    82.5%   82.49%   -0.01%     
==========================================
  Files         286      286              
  Lines       39112    39131      +19     
  Branches     5932     5936       +4     
==========================================
+ Hits        32268    32283      +15     
- Misses       5431     5433       +2     
- Partials     1413     1415       +2
Impacted Files Coverage Δ
sphinx/util/inspect.py 55.2% <100%> (+0.67%) ⬆️
tests/test_util_inspect.py 80.89% <66.66%> (-0.67%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 342de4f...17d32d6. Read the comment docs.

@tk0miya tk0miya changed the base branch from master to 1.8 Sep 5, 2018

@tk0miya tk0miya changed the base branch from 1.8 to master Sep 5, 2018

@tk0miya

LGTM with nits.
Could you rebase this into 1.8 branch?
Then I'll merge this soon.

@tk0miya tk0miya added the autodoc label Sep 5, 2018

@tk0miya tk0miya added this to the 1.8.0 milestone Sep 5, 2018

Ensure frozenset object descriptions are reproducible
Whilst working on the Reproducible Builds effort [0], we noticed
that sphinx could generate output that is not reproducible.

In particular, the rendering of `frozenset` objects in default
arguments and elsewhere is currently non-determinstic.

For example:

    frozenset(['a', 'b', 'c'])

Might be rendered as any of:

    frozenset({'a', 'b', 'c'})
    frozenset({'a', 'c', 'b'})
    frozenset({'b', 'a', 'c'})
    frozenset({'b', 'c', 'a'})
    frozenset({'c', 'a', 'b'})
    frozenset({'c', 'b', 'a'})

Patch attached that sorts the contents of frozensets whilst rendering.
This is parallel to the `dict` and `set` type logic

  [0] https://reproducible-builds.org/
@lamby

This comment has been minimized.

Show comment
Hide comment
@lamby

lamby Sep 5, 2018

Contributor

Rebased against 1.8 branch.

Contributor

lamby commented Sep 5, 2018

Rebased against 1.8 branch.

@tk0miya

tk0miya approved these changes Sep 5, 2018

@tk0miya tk0miya merged commit 750e037 into sphinx-doc:master Sep 5, 2018

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@tk0miya

This comment has been minimized.

Show comment
Hide comment
@tk0miya

tk0miya Sep 5, 2018

Member

Thanks a lot!

Member

tk0miya commented Sep 5, 2018

Thanks a lot!

tk0miya added a commit that referenced this pull request Sep 5, 2018

@lamby

This comment has been minimized.

Show comment
Hide comment
@lamby

lamby Sep 10, 2018

Contributor

Thanks @tk0miya. Do you know when this might hit a release? :)

Contributor

lamby commented Sep 10, 2018

Thanks @tk0miya. Do you know when this might hit a release? :)

@tk0miya

This comment has been minimized.

Show comment
Hide comment
@tk0miya

tk0miya Sep 10, 2018

Member

It will be released within a few days. Please watch #4986.

Member

tk0miya commented Sep 10, 2018

It will be released within a few days. Please watch #4986.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment