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

implemented read_from_file(filename) in config.py #38

Closed
wants to merge 4 commits into from

Conversation

raymond-van
Copy link

I noticed read_from_file(path) was on your TODO list so I went ahead and implemented it. We can now easily create custom configurations for more unique simulations.

read_from_file reads through the plaintext file custom_config.txt and parses each line and sets the corresponding configuration variable.

Great work on this repo and sorry on behalf of my fellow students for bombarding your repo with endless PRs.

@paulvangentcom
Copy link
Owner

Thanks Raymond, for adding functionality. I've been closing PRs left and right since not much actual functionality is added or they break things (although I've learned a thing or two glossing through them).

I like your implementation, but was thinking to implement the config through python's configparser from the stdlib.

If you like and have the time, could you consider adapting your PR to reflect this? Not much change should be needed and it will give some more functionality like sections which could be of use later. If you don't have the time, don't worry, in that case I'll update your PR later in the week.

Cheers

@raymond-van
Copy link
Author

Hey Paul,

Thanks for getting back!

I gave it a go at implementing read_from_file using configparser. I replaced the custom_config.txt file with an .ini file that contains sections. The code in read_from_file is essentially the same, just reworked so that it works with .ini files.

Let me know if this is what you were looking for. If not, I’ll try to fix it when I get the chance.

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.

None yet

2 participants