JDACS4C-IMPROVE / IMPROVE

Libraries and scripts for basic IMPROVE functionalities
MIT License
1 stars 3 forks source link

Work on docstrings for config #171

Open jonesse3 opened 1 day ago

jonesse3 commented 1 day ago

https://github.com/JDACS4C-IMPROVE/IMPROVE/tree/develop/improvelib/config

jonesse3 commented 1 day ago

@wilke @adpartin I started working on this but came across issues with the code. It's best if one of you tackle docstrings for this since there are deprecated parameters (not removed) and coding logic errors.

https://github.com/JDACS4C-IMPROVE/IMPROVE/blob/develop/improvelib/config/csa.py

Example of deprecated parameter:

sog.add_argument(
            '--learning_rate',
            metavar='LEARNING_RATE',
            dest="learning_rate",
            type=float,
            help="Learning rate for training. DEPRIECATED: Specify in model_params_per_dataset instead."
        )

Example of code that doesn't make sense to me:

    def _load_parsl_config(self):
        # if the config file is a yaml file load it as yaml
        if self.parsl_config_file.endswith('.yaml'):

            # print error not supported and exit
            logger.error("YAML format is not supported for parsl_config_file")
            sys.exit(1)

            with open(self.parsl_config_file, 'r') as f:
                return yaml.safe_load(f)
        # if the config file is a json file load it as json
        elif self.parsl_config_file.endswith('.json'):

            # print error not supported and exit
            logger.error("JSON format is not supported for parsl_config_file")
            sys.exit(1)

            with open(self.parsl_config_file, 'r') as f:
                return json.load(f)

        # if the config file is python file import it and assign the importet parsl_config to self.parsl_config
        elif self.parsl_config_file.endswith('.py'):
            # import the parsl_config from the file

            # add the directory of the config file to the path
            sys.path.append(os.path.dirname(self.parsl_config_file))

            # get filename without extension
            filename = os.path.basename(self.parsl_config_file).split('.')[0]

            # import the file

            importet_parsl_config = importlib.import_module(filename)

            # assign the parsl_config to self.parsl_config
            self.parsl_config = importet_parsl_config.parsl_config

        else:
            logger.error("Unknown file format for parsl_config_file")
            sys.exit(1)

I don't know if def _load_parsl_config(self): supports json or yaml files.

It seems that these files are still a work in progress as they are used in workflows/CSA.

jonesse3 commented 1 day ago

Since the code seems to support loading of YAML and JSON files, just remove or modify if file is not really YAML despite the file extension:

# print error not supported and exit
logger.error("YAML format is not supported for parsl_config_file")
sys.exit(1)