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

Not understanding the current way of handling draccus.load #32

Open
IrvingF7 opened this issue Feb 16, 2025 · 3 comments
Open

Not understanding the current way of handling draccus.load #32

IrvingF7 opened this issue Feb 16, 2025 · 3 comments

Comments

@IrvingF7
Copy link

IrvingF7 commented Feb 16, 2025

Thank you for this excellent project!

My understanding is that we can use draccus.load to load a local yaml/json file to overwrite some dataclass.

However, if we see the draccus.load defined here, we see that

def load(t: Type[Dataclass], stream):
    dictionary = load_config(stream)
    return decode(t, dictionary)
def load_config(stream, *, file=None):
    if file is not None:
        fpath = str(file)
        if fpath.endswith(".toml"):
            with config_type("toml"):
                return load_config(stream)
        elif fpath.endswith(".json"):
            with config_type("json"):
                return load_config(stream)
        elif fpath.endswith(".yaml") or fpath.endswith(".yml"):
            with config_type("yaml"):
                return load_config(stream)
    parser = Options.get_config_type().value
    return parser.load_config(stream)

We see that the load function does not pass a file into the load_config function, which would result in load_config processing the stream as a str, not a file stream.

The error message looks like this:

Traceback (most recent call last):
  File "{MYPATH}\draccus_check\train.py", line 151, in <module>
    main()
  File "{MYPATH}\draccus_check\train.py", line 148, in main
    train_cfg = draccus.load(TrainConfig, train_cfg.model_cfg_path)
  File "{MYPATH}\draccus_check\.venv\lib\site-packages\draccus\cfgparsing.py", line 38, in load
    return decode(t, dictionary)
  File "{MYPATH}\draccus_check\.venv\lib\site-packages\draccus\parsers\registry_utils.py", line 78, in wrapper
    return base_func(*args, **kw)
  File "{MYPATH}\draccus_check\.venv\lib\site-packages\draccus\parsers\decoding.py", line 48, in decode
    return get_decoding_fn(cls)(raw_value, ())  # type: ignore
  File "{MYPATH}\draccus_check\.venv\lib\site-packages\draccus\parsers\decoding.py", line 109, in decode_dataclass
    obj_dict: Dict[str, Any] = d.copy()
AttributeError: 'str' object has no attribute 'copy'

In argparsing.py, this is handled by first open the file stream

if config_path is not None:
    with open(config_path, "r") as f:
        file_args = cfgparsing.load_config(f, file=config_path)

And in my own experiment, I was able to do

with open('config/train/freeze_vision_tower.yaml', "r") as f:
    train_cfg = draccus.load(TrainConfig, f)

with both json and yaml config file, and I was able to verify that load indeed overwrites default config settings.

So, do we want to update the documentation or maybe refactor the codebase a bit?

I am happy to open a pull request to work on whichever direction you see fit

@dlwh
Copy link
Owner

dlwh commented Feb 18, 2025

Yeah I think this is inherited behavior I don't love.

My hunch is we should be more pythonic and do draccus.load vs draccus.loads. wdyt?

@IrvingF7
Copy link
Author

Yeah I think this is inherited behavior I don't love.

My hunch is we should be more pythonic and do draccus.load vs draccus.loads. wdyt?

Hi! Thanks for the reply.

By load vs. loads, do you mean something like this?

If so, I am happy to refactor the code and potentially work on the readme a bit to reflect the changes.

@dlwh
Copy link
Owner

dlwh commented Feb 18, 2025

Yeah I do, sorry for being obscure. I don't love that convention either, but it is very pythonic so it's probably the way to go.

I'd appreciate the PR, thank you!

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

No branches or pull requests

2 participants