search

Farama-Foundation / AutoROM

A tool to automate installing Atari ROMs for the Arcade Learning Environment
MIT License
77 stars 21 forks source link

RAR Download Method #3

Closed MarioJayakumar closed 4 years ago

MarioJayakumar commented 4 years ago

This PR switches the download scheme to download a single RAR file and extract the relevant ROMs. The only issue is that the user is required to have RAR installed on their system.

benblack769 commented 4 years ago

So it looks like python's patool package depends on the system's patool which in turn depends on (rar, unrar, or 7z) to be installed, and not one of these installations happens automatically. This is a bit ridiculous.

I don't think patool is a good idea if it just depends on something else to be installed. Also make sure that the python interface, if any, is in setup.py.

MarioJayakumar commented 4 years ago

So by interface, you mean command line args?

benblack769 commented 4 years ago

No. I mean that patoolib is imported in the python, and its not required in setup.py

benblack769 commented 4 years ago

So like, is there any python library that just depends on rar or unrar to be installed? This way, we could cut one middleman out of the picture.

MarioJayakumar commented 4 years ago

The only other one I know of is RarFile, but that one doesn't seem to have very good support. According to this: https://stackoverflow.com/questions/1185959/read-content-of-rar-file-into-memory-in-python , there is no pypi library that can install unrar for you

benblack769 commented 4 years ago

I guess my point is that the current method doesn't work well either, as I had to install 3 dependencies in sequence, by hand, in order to get the thing to work. I hope that with RarFile there will only be 1 thing to install by hand.

Either way, I would be a lot happier if there was a check for proper support before the download starts so you don't have to wait for it to download only for it to crash. Perhaps we can include a simple rar file in the repo and try to extract it just so the user gets the error message before download finishes?

Also, a download bar would be nice? https://stackoverflow.com/questions/40544123/how-to-use-tqdm-in-python-to-show-progress-when-downloading-data-online

MarioJayakumar commented 4 years ago

Ok so I can implement the test unrar and the progress bar now. Regarding patools, its not installable via pip? Adding that to the requirements should fix it right

benblack769 commented 4 years ago

So there is a python library of patools which is installable with pip, which in turn calls a system library for patools which is not.

Adding the python library to the requirements and the setup.py would be nice.

benblack769 commented 4 years ago

And yes, that sounds like a good plan to me.