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

Remove usage of sys.maxint #2

Closed
cezarsl opened this Issue Oct 15, 2013 · 5 comments

Comments

Projects
None yet
2 participants
@cezarsl

cezarsl commented Oct 15, 2013

In FuncSource.find_code_lines() there is a mention of sys.maxint. This doesn't work on Python 3.
Since the actual value is not used, I suggest replacing it with a hardcoded 2 ** 31 - 1, which will work on all versions of Python (sys.maxint is guaranteed to be at least that)

    def find_source_lines(self):
        """Mark all executable source lines in fn as executed 0 times."""
        strs = trace.find_strings(self.filename)
        lines = trace.find_lines_from_code(self.fn.__code__, strs)
        self.firstcodelineno = sys.maxint # replace here
        for lineno in lines:
            self.firstcodelineno = min(self.firstcodelineno, lineno)
            self.sourcelines.setdefault(lineno, 0)
        if self.firstcodelineno == sys.maxint: # and here
            self.firstcodelineno = self.firstlineno

P.S. If I'm not mistaken, and firstlineno is included in the set lines, then you can remove the lines that depend on that assignment :

    def find_source_lines(self):
        """Mark all executable source lines in fn as executed 0 times."""
        strs = trace.find_strings(self.filename)
        lines = trace.find_lines_from_code(self.fn.__code__, strs)
        for lineno in lines:
            self.firstcodelineno = min(self.firstcodelineno, lineno)
            self.sourcelines.setdefault(lineno, 0)

@ghost ghost assigned mgedmin Oct 16, 2013

@mgedmin

This comment has been minimized.

Show comment
Hide comment
@mgedmin

mgedmin Oct 16, 2013

Owner

I find that I don't remember the code at all. I'll take a look at it when I can find the time, hopefully soon!

Owner

mgedmin commented Oct 16, 2013

I find that I don't remember the code at all. I'll take a look at it when I can find the time, hopefully soon!

@mgedmin

This comment has been minimized.

Show comment
Hide comment
@mgedmin

mgedmin Oct 16, 2013

Owner

FWIW proper Python 3 support requires, in my mind, the following:

  • a tox.ini running the tests on all supported Python versions
  • trove classifiers in setup.py declaring the supported Python versions
  • sufficient test coverage to identify all Python 3 incompatibilities such as this one
Owner

mgedmin commented Oct 16, 2013

FWIW proper Python 3 support requires, in my mind, the following:

  • a tox.ini running the tests on all supported Python versions
  • trove classifiers in setup.py declaring the supported Python versions
  • sufficient test coverage to identify all Python 3 incompatibilities such as this one
@mgedmin

This comment has been minimized.

Show comment
Hide comment
@mgedmin
Owner

mgedmin commented Oct 16, 2013

@mgedmin

This comment has been minimized.

Show comment
Hide comment
@mgedmin

mgedmin Oct 16, 2013

Owner

Doctests were invented by the devil to make testing code hard.

Owner

mgedmin commented Oct 16, 2013

Doctests were invented by the devil to make testing code hard.

mgedmin added a commit that referenced this issue Oct 16, 2013

Add a test for @coverage
It fails on Python 3.x -- see issue #2.

@mgedmin mgedmin closed this in 58fb062 Oct 16, 2013

@mgedmin

This comment has been minimized.

Show comment
Hide comment
@mgedmin

mgedmin Oct 16, 2013

Owner

Released profilehooks 1.7 with this fix.

Owner

mgedmin commented Oct 16, 2013

Released profilehooks 1.7 with this fix.

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