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

pyflakes: remove unused imports #65175

Closed
vstinner opened this issue Mar 19, 2014 · 14 comments
Closed

pyflakes: remove unused imports #65175

vstinner opened this issue Mar 19, 2014 · 14 comments

Comments

@vstinner
Copy link
Member

BPO 20976
Nosy @warsaw, @vstinner, @bitdancer, @berkerpeksag, @serhiy-storchaka
Files
  • unused_imports.patch
  • issue20976_tarfile.diff
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2014-03-20.08:29:57.618>
    created_at = <Date 2014-03-19.10:47:40.422>
    labels = []
    title = 'pyflakes: remove unused imports'
    updated_at = <Date 2014-03-23.18:25:06.633>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2014-03-23.18:25:06.633>
    actor = 'python-dev'
    assignee = 'none'
    closed = True
    closed_date = <Date 2014-03-20.08:29:57.618>
    closer = 'vstinner'
    components = []
    creation = <Date 2014-03-19.10:47:40.422>
    creator = 'vstinner'
    dependencies = []
    files = ['34511', '34513']
    hgrepos = []
    issue_num = 20976
    keywords = ['patch']
    message_count = 14.0
    messages = ['214073', '214078', '214080', '214135', '214152', '214186', '214189', '214227', '214230', '214232', '214233', '214236', '214624', '214625']
    nosy_count = 8.0
    nosy_names = ['barry', 'vstinner', 'jnoller', 'r.david.murray', 'python-dev', 'sbt', 'berker.peksag', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'patch review'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue20976'
    versions = ['Python 3.5']

    @vstinner
    Copy link
    Member Author

    I ran pyflakes on Python 3.5. Attached patch removes unused imports.

    Sometimes, it's tricky to decide if an import is useless or if it is part of the API.

    Strange example using import to define a method!
    ---

    class Message:
        ...
        def get_charsets(self, failobj=None):
            ...
        # I.e. def walk(self): ...
        from email.iterators import walk

    For the email module, I moved "from quopri import decodestring as _qdecode" from Lib/email/utils.py to email submodules where it used.

    I made a similar change in multiprocessing for "from subprocess import _args_from_interpreter_flags".

    Since "_qdecode" and "_args_from_interpreter_flags" are private functions, I don't consider that they were part of the public API.

    Removing imports might reduce the Python memory footprint and speedup the Python startup.

    @berkerpeksag
    Copy link
    Member

    Here's an alternative patch for the tarfile module.

    @vstinner
    Copy link
    Member Author

    IMO issue20976_tarfile.diff is useless.

    @serhiy-storchaka
    Copy link
    Member

    LGTM. But for _qdecode you should ask RDM.

    @bitdancer
    Copy link
    Member

    I would prefer that _qdecode be left alone.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 20, 2014

    New changeset f6f691ff27b9 by Victor Stinner in branch '3.4':
    Issue bpo-20976: pyflakes: Remove unused imports
    http://hg.python.org/cpython/rev/f6f691ff27b9

    New changeset 714002a5c1b7 by Victor Stinner in branch 'default':
    (Merge 3.4) Issue bpo-20976: pyflakes: Remove unused imports
    http://hg.python.org/cpython/rev/714002a5c1b7

    @vstinner
    Copy link
    Member Author

    I would prefer that _qdecode be left alone.

    Ok, I leaved these symbols unchanged in Lib/email/utils.py even if they are not used:

    from quopri import decodestring as _qdecode
    from email.encoders import _bencode, _qencode

    @warsaw
    Copy link
    Member

    warsaw commented Mar 20, 2014

    On Mar 20, 2014, at 08:29 AM, STINNER Victor wrote:

    from quopri import decodestring as _qdecode
    from email.encoders import _bencode, _qencode

    AFAICT, _qdecode is only used in email/messages.py, so perhaps it's better to
    import it there and remove it from utils.py?

    _bencode and _qencode are imported/defined and used in encoders.py. It
    doesn't seem like utils.py or any other code uses them.

    They're all non-public so why not clean it up?

    @bitdancer
    Copy link
    Member

    Well, one reason is I was afraid mailman might be using them. So if you are cool with it, that removes that objection.

    The other reason was that it seemed they were being used "from" utils on purpose, as a design thing. I did not take the time to do a full analysis, since Victor wanted to get his patch in.

    So, if you've taken a look and you think there's no reason to keep them the way they are, then I'm fine with it.

    @vstinner
    Copy link
    Member Author

    Barry, David: It's up to you. I'm done with this issue, but you can drop more unused import if you want. Since I don't know well the email module, I don't want to be responsible of breaking it :-)

    @warsaw
    Copy link
    Member

    warsaw commented Mar 20, 2014

    On Mar 20, 2014, at 01:32 PM, R. David Murray wrote:

    Well, one reason is I was afraid mailman might be using them. So if you are
    cool with it, that removes that objection.

    Nope, neither the 2.1 or 3.0 code uses those methods AFAICT.

    The other reason was that it seemed they were being used "from" utils on
    purpose, as a design thing. I did not take the time to do a full analysis,
    since Victor wanted to get his patch in.

    I suspect it's just left over cruft from the early days of the email/mimelib
    code.

    So, if you've taken a look and you think there's no reason to keep them the
    way they are, then I'm fine with it.

    Do you want the honors? :)

    @bitdancer
    Copy link
    Member

    Sure.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 23, 2014

    New changeset 5d645f290d6a by R David Murray in branch '3.4':
    bpo-20976: remove unneeded quopri import in email.utils.
    http://hg.python.org/cpython/rev/5d645f290d6a

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 23, 2014

    New changeset d308c20bf2f4 by R David Murray in branch 'default':
    Merge bpo-20976: remove unneeded quopri import in email.utils.
    http://hg.python.org/cpython/rev/d308c20bf2f4

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    None yet
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants