Improve/revisit CallInfo.__repr__

[blueyed]

It shouldn't have literal newlines in it.

[RonnyPfannschmidt]

lovely just so much more nice

[RonnyPfannschmidt]

based on the final traceback, how about using "<CallInfo when={!r} {!r}: {}>".format(self.when, self.excinfo.type, self.excinfo.value)

soit would spell <CallInfo when='123' ZeroDivisionError: division by zero>

[blueyed]

Should use str(self.excinfo.type.__name__) then likely, and using str(self.excinfo.value) has the original issue, that it would contain literal "\n".

[blueyed]

I think just using repr(self.excinfo) is good - it is always good to use reprs in __repr__ after all.
We could drop the "exc: " prefix though? (Done.)

[blueyed]

@RonnyPfannschmidt

based on the final traceback

What do you mean here btw?

[blueyed]

Having repr(self.excinfo) there might be also good/better, since it would include tblen:
<ExceptionInfo AssertionError('tearDown\nassert 0') tblen=1>, but makes it a bit longer.
Having it as excinfo=<ExceptionInfo AssertionError('tearDown\nassert 0') tblen=1> therein would clearly indicate that it is a property, too.

[RonnyPfannschmidt]

i like it

wrt the trace-back, i was starting at the trace back in the pr about that character difference in the zerodivisionerror

[blueyed]

i like it

Which one? The whole repr of excinfo with excinfo= prefix?

[RonnyPfannschmidt]

yup

[blueyed]

Amended.

[nicoddemus]

Hmmm this definitely should include a changelog entry (improvement I think)

[blueyed]

@nicoddemus
IMHO it is just noise.
(if it was important to / used by people might have fixed/improved it already also maybe ;))

[nicoddemus]

Well it does change expected output... from someone who thinks any form of outward change to be possibly breaking, I'm suprised yout don't think this deserves a changelog entry.

[blueyed]

It's a minor internal improvement - I do not expect somebody to rely on parsing the __repr__ output or something similar really. And if so they would notice it anyway, before reading it in the changelog I assume.. ;)

[nicoddemus]

Well, I still think it does not cost us much to write a single file with a single line externing that change to our users. :)

[blueyed]

What's the benefit?
Just imagine 100 people reading it - that's maybe 500s "used" then.

[nicoddemus]

What's the benefit?

Well external changes, as small as they may be, might still break existing test suites and plugins. When that happens, it shows care from the maintainers if at least it is mentioned in the CHANGELOG.

[blueyed]

Well external changes, as small as they may be, might still break existing test suites and plugins.

Agreed.

When that happens, it shows care from the maintainers if at least it is mentioned in the CHANGELOG.

What is "careful" about being able to read in the changelog then something along "Improved __repr__ of X"? You see the error already.
And if you would check the changelog for plugin compatibility etc, you would not go and revisit your code/plugin to see if you are using that specific __repr__, would you? I.e. it would still only show in your tests etc.

Apart from that there's always git-log if you want to revisit all changes.

I am not reading changelogs myself mostly, so extra verbosity does not hurt me (except for having to write it twice (git log and changelog), and having to run CI for it twice usually - mainly because we're not squash-merging, and a lot of these things have no issue/PR number when submitting them etc).
(well, and I have CHANGELOG.rst and doc/en/announce in .ignore for rg, since I am interested in actual code when grepping usually only)

For me it really adds extra friction/work, and I think it is better to fix minor issues like this, and not just skip them because you have to add noise / extra work for "documenting" them.

[nicoddemus]

What is "careful" about being able to read in the changelog then something along "Improved repr of X"? You see the error already.

As a plugin writer, I like to know when things that might break my test suite are properly mentioned in the CHANGELOG. Not because I will preemtively act on them, but when I get a breakage I can consult the CHANGELOG and pinpoint the cause without having to spend sometime debugging and wondering if it is my code or environment.

I had this case recently in pytest-mock, when we changed the output of 1 extra items to 1 extra item, which broke a few tests. Of course I knew about the change before hand, but a normal user would very likely not know about it and might waste time investigating further. When we realize that this might happen with internal plugins in private companies which are part of very large code bases, then it makes even more sense to me.

I am not reading changelogs myself mostly

I don't have any data to back me up, but I'm sure most users at least glance at the CHANGELOG when they see the release announcement.

For me it really adds extra friction/work, and I think it is better to fix minor issues like this, and not just skip them because you have to add noise / extra work for "documenting" them.

Well I'm sorry if you feel this way, but I'm still convinced having a rich CHANGELOG demonstrates care by the maintainers and respect for the users. Feel free to start a vote/poll on the mailing list to change this policy, but I can add my 👎 in advance to that.

And actually it boggles my mind that adding a single file to a commit where you often spent a few hours or minutes is such a friction to you. I usually just add them as part of the first commit.