Skip to content

Conversation

@zmx27
Copy link
Collaborator

@zmx27 zmx27 commented Sep 10, 2025

No description provided.

@zmx27
Copy link
Collaborator Author

zmx27 commented Sep 10, 2025

@sbillinge ready for review

Copy link
Contributor

@sbillinge sbillinge left a comment

Choose a reason for hiding this comment

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

one small comment

"header": "f",
"h": "mask the edge pixels, first four means the number of pixels masked in each edge \
(left, right, top, bottom), the last one is the radius of a region masked around the corner",
"h": (
Copy link
Contributor

Choose a reason for hiding this comment

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

let's take this opportunity to rename variables to something more interpretable (wild guess here but this is a help message?). Same with n and d below (not help messages but rename.

As in the previous review, to separate possible breaking changes and linting changes, it is ok to put in a FIXME and return later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here is a segment from the beginning of the file:

# Text to display before the argument help
    _description = """Description of configurations
    """
    # Text to display after the argument help
    _epilog = """
    """

    """
    optdata contains these keys:
    these args will be passed to argparse, see the documents of argparse for
    detail information

    'f': full, (positional)
    's': short
    'h': help
    't': type
    'a': action
    'n': nargs
    'd': default
    'c': choices
    'r': required
    'de': dest
    'co': const
    """
    _optdatanamedict = {
        "h": "help",
        "t": "type",
        "a": "action",
        "n": "nargs",
        "d": "default",
        "c": "choices",
        "r": "required",
        "de": "dest",
        "co": "const",
    }

It seems like these variable names are deliberately named. They stand for help, nargs, and default respectively, but they seem to be eventually passed into the _optdatanamedict variable later and used. Do you still want me to rename them?

Copy link
Contributor

Choose a reason for hiding this comment

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

let's leave them then. More trouble than it is worth to rename them I think.

@sbillinge sbillinge merged commit 554a92c into diffpy:migration Sep 17, 2025
@sbillinge sbillinge deleted the fix-flake8-config branch September 17, 2025 02:45
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

Successfully merging this pull request may close these issues.

2 participants