Skip to content
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

stbt.FrameObject repr: Limit float property values to 3 decimal places #602

Closed
wants to merge 1 commit into from

Conversation

drothlis
Copy link
Contributor

@drothlis drothlis commented Jul 5, 2019

Otherwise doctest output of floating-point values might vary from run to
run.

60 frames per second is 17ms per frame, so a 1ms precision should be
enough for any measurements related to video-frames. For example we have
been affected by this in a FrameObject that we use in our A/V sync
tests, that has a property showing the lag or lead time from the audio
to the corresponding video frame.

We only apply this to properties that have "time" or "_secs" in the
name, because we don't know what other use-cases people are using
floating-point values for. If you need this, at least now you have the
option of working "time" or "_secs" into your property name.

This won't help properties that are lists of floats, etc. But it's a
start.

@drothlis
Copy link
Contributor Author

drothlis commented Jul 5, 2019

I tried solving this problem with numtest, but:

  • You have to put import numtest at the top level of the file. This isn't suitable for doctests in production files as you don't want to install numtest in the production environment.
  • It doesn't work with pytest:
        def check_output(self, want, got, optionflags):
            res = doctest.OutputChecker.check_output(self, want, got,
    >                                                optionflags)
    E       TypeError: unbound method check_output() must be called with NumTestOutputChecker instance as first argument (got LiteralsOutputChecker instance instead)
    
    /usr/lib/python2.7/dist-packages/_pytest/doctest.py:279: TypeError
    
  • It doesn't work with Python 3 (it's available via pip3, but doesn't work). Last commit was 3 years ago.
    /usr/local/lib/python3.6/dist-packages/numtest/__init__.py:11: in <module>
        import StringIO
    E   ModuleNotFoundError: No module named 'StringIO'
    

numtest's implementation is < 200 lines of code. It would be nice to implement this in pytest itself, so that you can enable it globally in pytest.ini instead of adding # doctest: +NUMBER to each line. numtest's license is MIT (same as pytest).

@drothlis
Copy link
Contributor Author

drothlis commented Jul 5, 2019

P.S. numtest only applies to doctests written manually; we'd still need something like this for the doctests generated by stbt auto-selftest.

@drothlis drothlis added the RFC label Jul 5, 2019
@drothlis
Copy link
Contributor Author

drothlis commented Jul 5, 2019

@wmanley what do you think of this idea?

@drothlis
Copy link
Contributor Author

drothlis commented Jul 5, 2019

Note: Even if you do something like this in your FrameObject:

@property
def xyz(self):
    return round(..., 3)

You can still get something like 0.23300000000000001.

Otherwise doctest output of floating-point values might vary from run to
run.

60 frames per second is 17ms per frame, so a 1ms precision should be
enough for any measurements related to video-frames. For example we have
been affected by this in a FrameObject that we use in our A/V sync
tests, that has a property showing the lag or lead time from the audio
to the corresponding video frame.

We only apply this to properties that have "time" or "_secs" in the
name, because we don't know what other use-cases people are using
floating-point values for. If you need this, at least now you have the
option of working "time" or "_secs" into your property name.

This won't help properties that are lists of floats, etc. But it's a
start.
@drothlis
Copy link
Contributor Author

drothlis commented Jul 9, 2019

It would be nice to implement [numtest] in pytest itself

I have implemented pytest-dev/pytest#5576

@wmanley
Copy link
Contributor

wmanley commented Jul 10, 2019

Otherwise doctest output of floating-point values might vary from run to run.

I don't think this is true. All the floating-point operations we perform should be deterministic. The real issue solved here is that doctest output varies between Python versions.

@drothlis
Copy link
Contributor Author

I'm not going to merge this, it'll be too magical. Just the thought of having to document when it does & doesn't apply makes me think it's a bad idea.

@drothlis drothlis closed this Sep 18, 2019
@drothlis drothlis deleted the frameobject-repr-floats branch September 18, 2019 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants