sbi icon indicating copy to clipboard operation
sbi copied to clipboard

Renamed types.py to aviod import clash when running tests

Open famura opened this issue 1 year ago • 2 comments

When running the tests using pytest in PyCharm (Community Edition 2023/3) I got the following error

Fatal Python error: init_import_size: Failed to import the site module
Python runtime state: initialized
Traceback (most recent call last):
  File "REDACTED/sbi_env/lib/python3.8/site.py", line 169, in addpackage
    exec(line)
  File "<string>", line 1, in <module>
  File "REDACTED/sbi_env/lib/python3.8/site-packages/__editable___sbi_0_22_0_finder.py", line 2, in <module>
    from importlib.machinery import ModuleSpec, PathFinder
  File "REDACTED/sbi_env/lib/python3.8/importlib/__init__.py", line 57, in <module>
    import types
  File "/home/muf2rng/Software/famura/sbi/sbi/types.py", line 5, in <module>
    from typing import NewType, Optional, Sequence, Tuple, TypeVar, Union
  File "REDACTED/sbi_env/lib/python3.8/typing.py", line 23, in <module>
    import contextlib
  File "REDACTED/sbi_env/lib/python3.8/contextlib.py", line 7, in <module>
    from types import MethodType
ImportError: cannot import name 'MethodType' from partially initialized module 'types' (most likely due to a circular import) (/home/muf2rng/Software/famura/sbi/sbi/types.py)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "REDACTED/sbi_env/lib/python3.8/site.py", line 580, in <module>
    main()
  File "REDACTED/sbi_env/lib/python3.8/site.py", line 567, in main
    known_paths = addsitepackages(known_paths)
  File "REDACTED/sbi_env/lib/python3.8/site.py", line 350, in addsitepackages
    addsitedir(sitedir, known_paths)
  File "REDACTED/sbi_env/lib/python3.8/site.py", line 208, in addsitedir
    addpackage(sitedir, name, known_paths)
  File "REDACTED/sbi_env/lib/python3.8/site.py", line 179, in addpackage
    import traceback
  File "REDACTED/sbi_env/lib/python3.8/traceback.py", line 5, in <module>
    import linecache
  File "REDACTED/sbi_env/lib/python3.8/linecache.py", line 11, in <module>
    import tokenize
  File "REDACTED/sbi_env/lib/python3.8/tokenize.py", line 32, in <module>
    import re
  File "REDACTED/sbi_env/lib/python3.8/re.py", line 124, in <module>
    import enum
  File "REDACTED/sbi_env/lib/python3.8/enum.py", line 2, in <module>
    from types import MappingProxyType, DynamicClassAttribute
  File "/home/muf2rng/Software/famura/sbi/sbi/types.py", line 5, in <module>
    from typing import NewType, Optional, Sequence, Tuple, TypeVar, Union
  File "REDACTED/sbi_env/lib/python3.8/typing.py", line 23, in <module>
    import contextlib
  File "REDACTED/sbi_env/lib/python3.8/contextlib.py", line 7, in <module>
    from types import MethodType
ImportError: cannot import name 'MethodType' from partially initialized module 'types' (most likely due to a circular import) (/home/muf2rng/Software/famura/sbi/sbi/types.py)

I strongly believe that this is due to the types.py module file in sbi. It look to me that the line import types in python3.8/importlib/__init__.py cause the clash.

After renaming it, the error was gone. The tests always ran from the command line.

What does this implement/fix? Explain your changes

  • Renamed types.py to sbi_types.py.
  • Added .idea to the .gitignore.

Does this close any currently open issues?

There was no issue on that, yet.

Any other comments?

I do not think that this is related to issue #743

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • [x] I have read and understood the contribution guidelines
  • [ ] I have commented my code, particularly in hard-to-understand areas
  • [ ] I have added tests that prove my fix is effective or that my feature works
  • [ ] I have reported how long the new tests run and potentially marked them with pytest.mark.slow.
  • [x] New and existing unit tests pass locally with my changes
  • [x] I performed linting and formatting as described in the contribution guidelines
  • [x] I rebased on main (or there are no conflicts with main)

famura avatar Mar 15 '24 12:03 famura

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 77.12%. Comparing base (4c2f71e) to head (bd85a92). Report is 14 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #993      +/-   ##
==========================================
+ Coverage   76.37%   77.12%   +0.74%     
==========================================
  Files          84       87       +3     
  Lines        6507     6554      +47     
==========================================
+ Hits         4970     5055      +85     
+ Misses       1537     1499      -38     
Flag Coverage Δ
unittests 77.12% <100.00%> (+0.74%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Mar 15 '24 12:03 codecov[bot]

I just noticed that the same error also appeared when running a tutorial notebook in PyCharm

famura avatar Mar 15 '24 12:03 famura

@Baschdl can you confirm this problem with PyCharm?

janfb avatar Mar 18 '24 16:03 janfb

@Baschdl can you confirm this problem with PyCharm?

I talked to him on Discord. He does not have that problem, however, he is using Mac and I am using Ubuntu. Nevertheless, your PyCharm versions are identical to the minor release

famura avatar Mar 19 '24 14:03 famura

Thanks for accepting this change even though it doesn't seem to affect many people. I just merged main (personal preference for this operation) and will wait for the check to run before merging the PR

famura avatar Mar 20 '24 08:03 famura

There are still changes in the notebooks if I look at the diff. But the notebooks should be changed by this PR, should they?

janfb avatar Mar 20 '24 09:03 janfb