Skip to content

Conversation

@rienafairefr
Copy link

Stumbled upon this repo,
This should solve the issue #8 with the hardcoded paths, using argparse to parse incoming CLI arguments.
Didn't really test, don't have data files handy, but this should work

arguments.py Outdated
import argparse

parser = argparse.ArgumentParser('SWE1R scripts ')
parser.add_argument('--out', description='directory where the output files are written', default='/tmp/swep1r')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we will probably have a lot of windows users, I'd like to get rid of the "/tmp/" folder altogether.
It would be fine with me if it somehow chose the platforms temp folder or output to a subfolder of the current directory by default.

(the scripts also lack the ability to generate the output folder if it doesn't exist yet, but that's probably a seperate issue)

@JayFoxRox
Copy link
Member

JayFoxRox commented Jan 17, 2018

Thanks a lot for looking into it, I'll test and continue review later.

(Also check-out the main project, which is a loader and re-implementation of the game)

Unknown added 2 commits January 17, 2018 21:14
help instead of description, duh
default to output/ directory in the current folder, and create the entered directory if it doesnt exist
@rienafairefr
Copy link
Author

Yes, I stumbled upon that main project, I'll try it out as well ^^
I've added some commits, it works fine now with the added step to create the dir if it doesnt exist :-)

@JayFoxRox
Copy link
Member

Sorry for the delay on this one; I'm still unhappy with the quality to be honest.
I don't like how this moves a basic operation such as argument parsing into a new file and that it automatically creates the folder, even when just doing "--help".

I started fixing this locally but got distracted with other things (hence the delay). I'll try to get back to this soon though.

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

Successfully merging this pull request may close these issues.

2 participants