omni-us / jsonargparse

Implement minimal boilerplate CLIs derived from type hints and parse from command line, config files and environment variables
https://jsonargparse.readthedocs.io
MIT License
314 stars 42 forks source link

Wrong error message when using `@classmethod` with class_from_function. #454

Closed CM-BF closed 6 months ago

CM-BF commented 6 months ago

🐛 Bug report

From issue 309

When the classmethod's parameters do not include their type, the error message indicates the failure to find the method instead of prompting the lack of types.

Error message:

usage: data_manager.py [-h] [--test CONFIG]
                       [--test.dataset.help CLASS_PATH_OR_NAME]
                       --test.dataset DATASET
error: Parser key "test.dataset":
  Problem with given class_path '__main__.CalendarFromStr':
    type object 'type' has no attribute 'from_str'

To reproduce

class Calendar:
    def __init__(self, firstweekday: int):
        self.firstweekday = firstweekday

    @classmethod
    def from_str(cls, firstweekday): # --- When `: str` is missing ---
        if firstweekday == "Monday":
            return cls(1)

CalendarFromStr = class_from_function(Calendar.from_str, Calendar, name="CalendarFromStr")
class TestDataset:
    def __init__(self, dataset: CalendarFromStr):
        self.dataset = dataset

if __name__ == '__main__':
    parser = ArgumentParser(parser_mode='omegaconf')
    parser.add_class_arguments(TestDataset, 'test')
    cfg = parser.parse_path('test.yaml')
    print(cfg.as_dict())
    cfg = parser.instantiate_classes(cfg)

Config file

test:
  dataset:
    class_path: CalendarFromStr
    init_args:
      firstweekday: Monday

-->

Expected behavior

Indicate the missing of the type of firstweekday.

Environment

CM-BF commented 6 months ago

I also tried setting the type of dataset to a super class of Calendar:

def __init__(self, dataset: CalendarBase):

When I wrongly define the return_class type as:

class_from_function(Calendar.from_str, AnotherClass, name="CalendarFromStr") # AnotherClass instead of CalendarBase or Calendar

instead of telling me the class mismatch between CalendarFromStr and CalendarBase, I get Expected a dot import path string: CalendarFromStr.

Therefore, these two cases indicate that when there is something wrong, it always believes that there is something missing.

mauvilsa commented 6 months ago

Thank you for reporting! This has been fixed with pull request #458.