Skip to content

Fix tutorial: rename attr to urwid_attr #653

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

Merged
merged 1 commit into from
Sep 29, 2023

Conversation

penguinolog
Copy link
Collaborator

Prevent stdlib attr shadowing in case of sys.path altered

Fix: #649

Checklist
  • I've ensured that similar functionality has not already been implemented
  • I've ensured that similar functionality has not earlier been proposed and declined
  • I've branched off the master or python-dual-support branch
  • I've merged fresh upstream into my branch recently
  • I've ran tox successfully in local environment
  • I've included docstrings and/or documentation and/or examples for my code (if this is a new feature)

Prevent stdlib `attr` shadowing in case of `sys.path` altered
@github-actions github-actions bot added the docs Issues related to documentation label Sep 29, 2023
@penguinolog
Copy link
Collaborator Author

penguinolog commented Sep 29, 2023

@ulidtko without this PR changes, full regeneration of images in documentation is impossible :-(

Copy link
Collaborator

@wardi wardi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting, is there something special about the name attr?

@penguinolog
Copy link
Collaborator Author

interesting, is there something special about the name attr?

I mixed in my memory dataclasses with "attrs" package.
attr is a part of attrs package, it is extremely common library for cases, where dataclasses are not available or not enough functional.
You define like

import attr

@attr.s
class Config:
    a: dict[str, int] = attr.ib(factory=dict, ...

And other code is generated

@wardi
Copy link
Collaborator

wardi commented Sep 29, 2023

so this change isn't actually required then

@penguinolog
Copy link
Collaborator Author

so this change isn't actually required then

Scripts location is added to sys.path at first position, then during imports modules lookup uses sys.path. Anything using attr is broken and it's one of our dependencies, which call some logic on import

@wardi
Copy link
Collaborator

wardi commented Sep 29, 2023

the docs/tutorial directory isn't added to sys.path and these are scripts not modules to be imported, right?

@penguinolog
Copy link
Collaborator Author

the docs/tutorial directory isn't added to sys.path and these are scripts not modules to be imported, right?

It's added automatically on run as script.
Short example

alekseis@1857be7-lcelt ~/tmp $ batcat *          
───────┬──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
       │ File: logging.py
───────┼──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1   │ print("this should not be executed")
───────┴──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
───────┬──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
       │ File: tst.py
───────┼──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1   │ import logging
   2   │ import sys
   3   │ 
   4   │ print(sys.path)
   5   │ 
   6   │ logging.basicConfig(level=logging.INFO)
   7   │ logger = logging.getLogger(__name__)
   8   │ logger.info("it works")
───────┴──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
alekseis@1857be7-lcelt ~/tmp $ python tst.py     
this should not be executed
['/home/alekseis/tmp', '/usr/lib/python311.zip', '/usr/lib/python3.11', '/usr/lib/python3.11/lib-dynload', '/usr/local/lib/python3.11/dist-packages', '/usr/lib/python3/dist-packages', '/usr/lib/python3.11/dist-packages']
Traceback (most recent call last):
  File "/home/alekseis/tmp/tst.py", line 6, in <module>
    logging.basicConfig(level=logging.INFO)
    ^^^^^^^^^^^^^^^^^^^
AttributeError: module 'logging' has no attribute 'basicConfig'

As you can see, sys.path was altered and instead of normal logging, another code was imported

@penguinolog penguinolog merged commit 9dcafca into urwid:master Sep 29, 2023
@penguinolog penguinolog deleted the attr_shadow_attr branch September 29, 2023 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Issues related to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[docs] docs/tutorial/attr.py break tutorial load if trio installed
2 participants