Skip to content

Conversation

mgorny
Copy link
Contributor

@mgorny mgorny commented May 26, 2020

Python 3.9 finally removed sys.setcheckinterval(). While the package
apparently tried to account for that, the logic is flawed and the second
getattr() raises an AttributeError even if its result is never used.
This caused tests to fail:

  File "/tmp/tqdm/tqdm/tests/tests_tqdm.py", line 126, in pretest
    getattr(sys, 'setswitchinterval', getattr(sys, 'setcheckinterval'))(100)
AttributeError: module 'sys' has no attribute 'setcheckinterval'

Refactor the code into a try/except construct that does not execute
the setcheckinterval() branch unless setswitchinterval() is actually
missing. While at it, scale the arguments a bit -- the current version
used either 100 instructions or 100 seconds that were rather of very
different magnitudes.

python version:

3.9.0b1 (default, May 19 2020, 09:27:48) 
[GCC 9.2.0] linux
  • I have marked all applicable categories:
    • exception-raising fix
    • visual output fix
    • documentation modification
    • new feature
  • If applicable, I have mentioned the relevant/related issue(s)

Less important but also useful:

  • I have visited the source website, and in particular
    read the known issues
  • I have searched through the issue tracker for duplicates
  • I have mentioned version numbers, operating system and
    environment, where applicable:
    import tqdm, sys
    print(tqdm.__version__, sys.version, sys.platform)

Python 3.9 finally removed sys.setcheckinterval().  While the package
apparently tried to account for that, the logic is flawed and the second
getattr() raises an AttributeError even if its result is never used.
This caused tests to fail:

      File "/tmp/tqdm/tqdm/tests/tests_tqdm.py", line 126, in pretest
        getattr(sys, 'setswitchinterval', getattr(sys, 'setcheckinterval'))(100)
    AttributeError: module 'sys' has no attribute 'setcheckinterval'

Refactor the code into a try/except construct that does not execute
the setcheckinterval() branch unless setswitchinterval() is actually
missing.  While at it, scale the arguments a bit -- the current version
used either 100 instructions or 100 seconds that were rather of very
different magnitudes.
@mgorny mgorny requested a review from casperdcl as a code owner May 26, 2020 19:51
@codecov-commenter
Copy link

Codecov Report

Merging #978 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #978   +/-   ##
=======================================
  Coverage   87.08%   87.08%           
=======================================
  Files          21       21           
  Lines        1254     1254           
  Branches      215      215           
=======================================
  Hits         1092     1092           
  Misses        141      141           
  Partials       21       21           

@casperdcl casperdcl self-assigned this May 28, 2020
@casperdcl casperdcl added this to the v5.0.0 milestone May 28, 2020
@casperdcl casperdcl added c1-quick 🕐 Complexity low eol ☠ Deprecations that are wont-fix candidates to-merge ↰ Imminent labels May 28, 2020
@casperdcl casperdcl changed the base branch from master to devel June 3, 2020 09:31
@casperdcl casperdcl merged commit dcd18b3 into tqdm:devel Jun 3, 2020
@casperdcl
Copy link
Member

thanks!

@casperdcl casperdcl mentioned this pull request Jun 3, 2020
casperdcl added a commit that referenced this pull request Jun 3, 2020
- fix missing `sys.setcheckinterval` in py3.9 (#978)
- fix `keras.TqdmCallback` compatibility with `tensorflow==2.2.0` (#979)
- update documentation
  + correct `contrib.concurrent` correct `max_workers` (#977)
  + drop prominent mention of `xrange` (#965)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c1-quick 🕐 Complexity low eol ☠ Deprecations that are wont-fix candidates to-merge ↰ Imminent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants