Per-package path formatting, and backwards compatibility with CLI arguments #12
Conversation
JesseFarebro
changed the title
JesseFarebro
changed the title
|
I'm not exactly thrilled with there being two different install formats forever. And the new format is definitely better. Can you keep the new, simple install format here and make a PR to PettingZoo which supports both formats (looks at the old loction if the new one does not exist)? If you don't want to make a pettingzoo PR, then I can do so soon (hopefully). |
|
I.e. autorom should not have an install-dir formatting, and it should install the same to both multi_agent_ale_py and ale_py (as currently in master). |
|
I think the install format is useful, e.g., there have been times I wish I had I'm not opposed to changing the current ROM format for MA ale-py, I could create a PR @ PettingZoo no problem, I'd just like to keep the general formatting CLI flag. |
|
Yeah, I don't want to allow users to make these decisions unless absolutely necessary. The reason standardization is good is composability, that even when developers build new things on top of AutoROM (using the install-dir argument or something), downstream users can expect a similar layout of the files. |
|
|
||
| # Try and find multi-agent-ale-py | ||
| try: | ||
| # Assume all ROMs are supported | ||
| installation_dirs.append( | ||
| SupportedPackage( | ||
| resources.files("multi_agent_ale_py") / "roms", lambda _: True | ||
| resources.files("multi_agent_ale_py") / "roms", |
There was a problem hiding this comment.
MA ale-py format is the same as ale-py.
| packages = [SupportedPackage(pathlib.Path(install_dir), lambda _: True)] | ||
| packages = [ | ||
| SupportedPackage( | ||
| pathlib.Path(install_dir), "{rom}.bin", lambda _: True |
There was a problem hiding this comment.
User install format is just {rom}.bin.
JesseFarebro
changed the title
|
Ok, I think I am fairly happy with this. Just waiting for Justin to get back to us regarding the error message, I think. PettingZoo 1.11.2 has been released with the path support. |
|
Thanks for making these changes. These really are huge improvements to the project. |
-v/-y/--accept-licensewhich is now backwards compatible-d/--install-dirwhich is now backwards compatible--quietflag to suppress the install log