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

filecmp.dircmp does not allow non-shallow comparisons #57141

Open
kesmit mannequin opened this issue Sep 7, 2011 · 16 comments
Open

filecmp.dircmp does not allow non-shallow comparisons #57141

kesmit mannequin opened this issue Sep 7, 2011 · 16 comments
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@kesmit
Copy link
Mannequin

kesmit mannequin commented Sep 7, 2011

BPO 12932
Nosy @terryjreedy, @rbtcollins, @merwok, @regebro, @mitar, @cjerdonek
Files
  • issue12932.diff: Add input parameter 'shallow' to dircmp.
  • 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 = None
    created_at = <Date 2011-09-07.16:01:35.750>
    labels = ['type-feature', 'library']
    title = 'filecmp.dircmp does not allow non-shallow comparisons'
    updated_at = <Date 2018-01-03.09:42:28.161>
    user = 'https://bugs.python.org/kesmit'

    bugs.python.org fields:

    activity = <Date 2018-01-03.09:42:28.161>
    actor = 'mitar'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Library (Lib)']
    creation = <Date 2011-09-07.16:01:35.750>
    creator = 'kesmit'
    dependencies = []
    files = ['30589']
    hgrepos = []
    issue_num = 12932
    keywords = ['patch']
    message_count = 9.0
    messages = ['143692', '156736', '160966', '161823', '164648', '166246', '191154', '247695', '247696']
    nosy_count = 8.0
    nosy_names = ['terry.reedy', 'rbcollins', 'eric.araujo', 'lregebro', 'mitar', 'chris.jerdonek', 'planet36', 'kesmit']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue12932'
    versions = ['Python 3.6']

    Linked PRs

    @kesmit
    Copy link
    Mannequin Author

    kesmit mannequin commented Sep 7, 2011

    While filecmp.cmp and filecmp.cmpfiles allow a shallow option to be specified to invoke a more involved comparison of files, filecmp.dircmp does not. It is limited to shallow-only comparisons.

    This could be solved quite easily by adding a shallow keyword option to dircmp then changing the phase3 method to the following.

        def phase3(self): # Find out differences between common files
            xx = cmpfiles(self.left, self.right, self.common_files, self.shallow)
            self.same_files, self.diff_files, self.funny_files = xx

    @kesmit kesmit mannequin added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Sep 7, 2011
    @merwok
    Copy link
    Member

    merwok commented Mar 25, 2012

    Thanks for the report. Unfortunately 2.7 is closed to new features and the module is removed in 3.x, so there is nothing to do here.

    @merwok merwok closed this as completed Mar 25, 2012
    @regebro
    Copy link
    Mannequin

    regebro mannequin commented May 17, 2012

    filecmp is still there in Python 3.3 Alpha 3. I can't find any reference to it being deprecated.

    @terryjreedy
    Copy link
    Member

    Lennart, I saw your response on StackOverflow ;-).

    @terryjreedy terryjreedy reopened this May 29, 2012
    @merwok merwok changed the title dircmp does not allow non-shallow comparisons filecmp.dircmp does not allow non-shallow comparisons Jun 1, 2012
    @cjerdonek
    Copy link
    Member

    +1 for this.

    Whether or not this feature is implemented, I think the documentation should state that directory comparisons are done using "shallow=True". I created bpo-15250 for this.

    @cjerdonek
    Copy link
    Member

    Allowing dircmp() to accept a file comparison function is another option to consider that may address more needs going forward. shallow=False could be achieved by passing lambda a, b: filecmp.cmp(a, b, shallow=False).

    @planet36
    Copy link
    Mannequin

    planet36 mannequin commented Jun 14, 2013

    Add input parameter 'shallow' to dircmp.
    Use it in phase3 and phase4.
    Document it in filecmp.rst.

    Did not modify test_filecmp.py.

    @rbtcollins
    Copy link
    Member

    Thanks for the patch @planet36, however I think this is sufficiently large a change that we should also have a test case for it.

    I'm also retargeting this to the current open branches for feature work - 3.6.

    @rbtcollins
    Copy link
    Member

    Bah, wrong stage. patch review.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @jnm27
    Copy link

    jnm27 commented Nov 17, 2022

    @rbtcollins any update here?

    @merwok
    Copy link
    Member

    merwok commented Nov 17, 2022

    You can see that the latest comment is from 2015.

    Someone would need to take the patch, apply it to Python main branch, make sure tests, docs and changelog are present, and open a pull request. The devguide has help about that.

    aunzat added a commit to aunzat/cpython that referenced this issue Sep 25, 2023
    Co-authored-by: Steve Ward <[email protected]>
    Co-authored-by: Sanyam Khurana <[email protected]>
    aunzat added a commit to aunzat/cpython that referenced this issue Sep 26, 2023
    Co-authored-by: Steve Ward <[email protected]>
    Co-authored-by: Sanyam Khurana <[email protected]>
    aunzat added a commit to aunzat/cpython that referenced this issue Sep 27, 2023
    Co-authored-by: Steve Ward <[email protected]>
    Co-authored-by: Sanyam Khurana <[email protected]>
    encukou pushed a commit that referenced this issue Mar 4, 2024
    Co-authored-by: Steve Ward <[email protected]>
    Co-authored-by: Sanyam Khurana <[email protected]>
    woodruffw pushed a commit to woodruffw-forks/cpython that referenced this issue Mar 4, 2024
    @encukou
    Copy link
    Member

    encukou commented Mar 6, 2024

    Thanks to everyone involved!

    @encukou encukou closed this as completed Mar 6, 2024
    @merwok
    Copy link
    Member

    merwok commented Mar 6, 2024

    Really wish github would send a notif on the issue when a PR is created to solve it!

    @terryjreedy
    Copy link
    Member

    Not a bad idea. Did bpo do so? (I cannot remember either way for sure.)

    Given the absence of such notices, the PR author should have requested a review from you. If not already, the devguide should say something about looking for participants (preferably recent) who are members of 'Python' or 'python/core'. (bpo starred our names on comments.)

    adorilson pushed a commit to adorilson/cpython that referenced this issue Mar 25, 2024
    diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
    @JelleZijlstra
    Copy link
    Member

    Should the new parameter be made keyword-only? I feel we usually do that for newly added optional parameters. We have a little bit of time left before the RC phase to change this.

    @JelleZijlstra JelleZijlstra reopened this Jul 13, 2024
    JelleZijlstra added a commit to JelleZijlstra/cpython that referenced this issue Jul 14, 2024
    It is our general practice to make new optional parameters keyword-only,
    even if the existing parameters are all positional-or-keyword. Passing
    this parameter as positional would look confusing and could be error-prone
    if additional parameters are added in the future.
    @JelleZijlstra
    Copy link
    Member

    Submitted #121767 to make the new parameter keyword-only.

    JelleZijlstra added a commit that referenced this issue Jul 14, 2024
    It is our general practice to make new optional parameters keyword-only,
    even if the existing parameters are all positional-or-keyword. Passing
    this parameter as positional would look confusing and could be error-prone
    if additional parameters are added in the future.
    miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jul 14, 2024
    …ythonGH-121767)
    
    It is our general practice to make new optional parameters keyword-only,
    even if the existing parameters are all positional-or-keyword. Passing
    this parameter as positional would look confusing and could be error-prone
    if additional parameters are added in the future.
    (cherry picked from commit 50eec50)
    
    Co-authored-by: Jelle Zijlstra <[email protected]>
    JelleZijlstra added a commit that referenced this issue Jul 14, 2024
    …GH-121767) (#121777)
    
    It is our general practice to make new optional parameters keyword-only,
    even if the existing parameters are all positional-or-keyword. Passing
    this parameter as positional would look confusing and could be error-prone
    if additional parameters are added in the future.
    (cherry picked from commit 50eec50)
    
    Co-authored-by: Jelle Zijlstra <[email protected]>
    estyxx pushed a commit to estyxx/cpython that referenced this issue Jul 17, 2024
    …ython#121767)
    
    It is our general practice to make new optional parameters keyword-only,
    even if the existing parameters are all positional-or-keyword. Passing
    this parameter as positional would look confusing and could be error-prone
    if additional parameters are added in the future.
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    7 participants