Skip to content

Conversation

@ChristopherChudzicki
Copy link
Collaborator

This makes pytest run the doc/*.md files as doc tests, and fixes a few broken tests.

To facilitate this, I made ObjectWithSchema.__repr__ pretty-format its config dictionary before printing. Primarily, the goal there was to make sure keys are displayed in alphabetical order (which pprint does for dictionaries).

@ChristopherChudzicki
Copy link
Collaborator Author

ChristopherChudzicki commented Jun 24, 2019

@jolyonb Came across this issue when playing with python 3 today.

BTW: I suppose that there are cases where pretty-printing could be annoying by default, but in those cases I think our existing print method is already overwhelming. Anyway, I could move the pretty version into a separate method if you prefer. (I think it should be on by default, see comment below.)

@ChristopherChudzicki
Copy link
Collaborator Author

@jolyonb As I mentioned, my primary goal in pretty-formatting the ObjectWithSchema config during __repr__ was to alphabetize the keys. This is also going to help with py2/py3 compatibility since dictionary key order can be different in the two versions. (Dicts have a guaranteed print order in 3.7+, but not below.)

>>> print({letter: letter for letter in 'abcd'})
{'a': 'a', 'b': 'b', 'c': 'c', 'd': 'd'} # in 3.7
{'a': 'a', 'c': 'c', 'b': 'b', 'd': 'd'} # in 2.7, on my machine anyway

Copy link
Collaborator

@jolyonb jolyonb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looks fine to me.

@jolyonb jolyonb merged commit 26e40a6 into master Jun 24, 2019
@jolyonb
Copy link
Collaborator

jolyonb commented Jun 24, 2019

Just out of interest, you're using the master fork again?

@jolyonb jolyonb deleted the test-docs branch June 24, 2019 05:29
@ChristopherChudzicki
Copy link
Collaborator Author

ChristopherChudzicki commented Jun 24, 2019

Just out of interest, you're using the master fork again?

@jolyonb Hmmm my remotes were messed up. Fixed them.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants