Skip to content

style: fix flake8 errors for config.py #10

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.

Sign up for GitHub

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 17, 2025
Merged

style: fix flake8 errors for config.py #10

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 34 additions & 36 deletions src/diffpy/srxconfutils/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,34 +22,22 @@

import argparse
import os
import re
import sys
from configparser import ConfigParser
from functools import partial

try:
from collections import OrderedDict
except:
except Exception:
from ordereddict import OrderedDict

from diffpy.srxconfutils.tools import (
FakeConfigFile,
StrConv,
_configPropertyR,
_configPropertyRad,
_configPropertyRW,
opt2Str,
str2bool,
str2Opt,
)
from diffpy.srxconfutils.tools import FakeConfigFile, StrConv, opt2Str, str2Opt


class ConfigBase(object):
"""_optdatalist_default, _optdatalist are metadata used to
initialize the options, see below for examples.

options presents in --help (in cmd), config file, headers have same order as
in these list, so arrange them in right order here.
options presents in --help (in cmd), config file, headers have same order
as in these list, so arrange them in right order here.

optional args to control if the options presents in args, config file or
file header
Expand Down Expand Up @@ -128,7 +116,10 @@ class ConfigBase(object):
"sec": "Control",
"config": "n",
"header": "n",
"h": "create a config file according to default or current values",
"h": (
"create a config file according to "
"default or current values"
),
"d": "",
},
],
Expand Down Expand Up @@ -227,8 +218,12 @@ class ConfigBase(object):
"sec": "Others",
"config": "f",
"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

@sbillinge sbillinge Sep 12, 2025

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

@zmx27 zmx27 Sep 15, 2025

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

@sbillinge sbillinge Sep 17, 2025

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.

"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"
),
"n": 5,
"d": [1, 1, 1, 1, 50],
},
Expand Down Expand Up @@ -265,7 +260,7 @@ def __init__(self, filename=None, args=None, **kwargs):

# update config, first detect if a default config should be load
filename = self._findDefaultConfigFile(filename, args, **kwargs)
rv = self.updateConfig(filename, args, **kwargs)
self.updateConfig(filename, args, **kwargs)
return

# example, overload it
Expand All @@ -292,9 +287,9 @@ def _findConfigFile(self, filename=None, args=None, **kwargs):
:return: name of config file if found, otherwise None
"""
rv = None
if filename != None:
if filename is not None:
rv = filename
if args != None:
if args is not None:
if ("--configfile" in args) or ("-c" in args):
obj = self.args.parse_args(args)
rv = obj.configfile
Expand All @@ -316,9 +311,9 @@ def _findDefaultConfigFile(self, filename=None, args=None, **kwargs):
return: name of config file if found, otherwise None
"""
rv = self._findConfigFile(filename, args, **kwargs)
if rv == None:
if rv is None:
for dconf in self._defaultdata["configfile"]:
if (os.path.exists(dconf)) and (rv == None):
if (os.path.exists(dconf)) and (rv is None):
rv = dconf
return rv

Expand Down Expand Up @@ -499,7 +494,7 @@ def _copyConfigtoSelf(self, optnames=None):
value copied from self.config to self.*option*'. Set None to
update all
"""
if optnames != None:
if optnames is not None:
optnames = optnames if isinstance(optnames, list) else [optnames]
else:
optnames = []
Expand All @@ -521,7 +516,7 @@ def _copySelftoConfig(self, optnames=None):
copied from self.*option* to self.config. Set None to update
all
"""
if optnames != None:
if optnames is not None:
optnames = optnames if isinstance(optnames, list) else [optnames]
else:
optnames = []
Expand Down Expand Up @@ -575,7 +570,7 @@ def parseConfigFile(self, filename):

:param filename: str, file name of config file (include path)
"""
if filename != None:
if filename is not None:
filename = os.path.abspath(filename)
if os.path.exists(filename):
self.configfile = filename
Expand Down Expand Up @@ -607,16 +602,16 @@ def updateConfig(self, filename=None, args=None, **kwargs):
self._preUpdateConfig(**kwargs)

filename = self._findConfigFile(filename, args, **kwargs)
if filename != None:
if filename is not None:
rv = self.parseConfigFile(filename)
if args != None:
if args is not None:
rv = self.parseArgs(args)
if kwargs != {}:
rv = self.parseKwargs(**kwargs)

if (
(filename == None)
and ((args == None) or (args == []))
(filename is None)
and ((args is None) or (args == []))
and (kwargs == {})
):
rv = self._updateSelf()
Expand All @@ -642,10 +637,12 @@ def _postUpdateConfig(self, **kwargs):
def _createConfigFile(self):
"""Write output config file if specified in configuration the
filename is specified by self.createconfig."""
if (self.createconfig != "") and (self.createconfig != None):
if (self.createconfig != "") and (self.createconfig is not None):
self.writeConfig(self.createconfig, "short")
self.createconfig = ""
if (self.createconfigfull != "") and (self.createconfigfull != None):
if (self.createconfigfull != "") and (
self.createconfigfull is not None
):
self.writeConfig(self.createconfigfull, "full")
self.createconfigfull = ""
return
Expand Down Expand Up @@ -711,7 +708,7 @@ def getHeader(self, title=None, mode="full"):

lines = []
title = "# %s #" % (
self._defaultdata["headertitle"] if title == None else title
self._defaultdata["headertitle"] if title is None else title
)
lines.append(title)
# func decide if write the option to header according to mode
Expand Down Expand Up @@ -756,7 +753,7 @@ def resetDefault(self, optnames=None):
:param optnames: list of str, name of options to reset, None for
all options
"""
if optnames == None:
if optnames is None:
optnames = self._optdata.keys()
for optname in optnames:
if optname in self._optdata:
Expand All @@ -765,7 +762,8 @@ def resetDefault(self, optnames=None):
return

###########################################################################
# IMPORTANT call this method if you want to add options as class attributes!!!
# IMPORTANT call this method if you want to add
# options as class attributes!!!

@classmethod
def initConfigClass(cls):
Expand Down