search

facebookresearch / nocturne

A data-driven, fast driving simulator for multi-agent coordination under partial observability.
MIT License
259 stars 29 forks source link

[Feature] Don't install `cfgs` module as a top level Python import #63

Open samuela opened 1 year ago

samuela commented 1 year ago

Feature

In the process of packaging nocturne for Nixpkgs (https://github.com/NixOS/nixpkgs/pull/208847#issuecomment-1374957894), it was discovered that nocturne doesn't just install "nocturne" as a top-level package -- eg import nocturne -- but also "cfgs". In fact, having cfgs available on the top-level is required in order to import nocturne: https://github.com/facebookresearch/nocturne/blob/ae0a4e361457caf6b7e397675cc86f46161405ed/nocturne/__init__.py#L27

Is this intentional? Polluting the top-level scope with "cfgs" seems undesirable. Would it be possible to move cfgs out of the top-level package namespace?

eugenevinitsky commented 1 year ago

Thanks for flagging this! @xiaomengy @nathanlct do you see any reason this needs to be a top level folder? I don't.

nathanlct commented 1 year ago

That folder seems to be mostly Hydra configs associated with the examples/ folder (and the paths are still eugenevinitsky/), that's probably why it's a top folder. But why are those paths PROCESSED_TRAIN_NO_TL hardcoded in and imported from config.py, and not loaded from config.yml?

eugenevinitsky commented 1 year ago

@samuela we'll submit a PR moving this out, do you have a timeline you need this on?

samuela commented 1 year ago

Thanks @eugenevinitsky! No rush on my end!