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

Question about writing modules #26

Open
dylanmikesell opened this issue Nov 28, 2020 · 1 comment
Open

Question about writing modules #26

dylanmikesell opened this issue Nov 28, 2020 · 1 comment

Comments

@dylanmikesell
Copy link

dylanmikesell commented Nov 28, 2020

First off, thank you for making this code public! I have a question about the goal of this code though.

Is there a reason that this code has been written in such a way that modularizing and using functions as a library was not done? For example, in run_fp.py there is the line

	# Fingerprinting
	chdir('fingerprint')
	for param in fp_params:
		print("Fingerprinting %s" % param)
		process = subprocess.Popen((fpCommand % param),
			stdout=subprocess.PIPE, shell=True)
		output, error = process.communicate()
		print(output.decode('UTF-8'))

there are numerous calls using subprocess throughout this code. If things were written modularly the above code could be replaced by

from fingerprint import gen_fp
...
	# Fingerprinting
	for param in fp_params:
		print("Fingerprinting %s" % param)
		gen_fp.call_mad(param)
		gen_fp.run_fingerprint(param)

where these are two functions within gen_fp.py that can do what the command line call to gen_fp.py does without using the subprocess library to run things from the command line.

Using things like chdir() make this code practically impossible to use outside of the very prescribe directory structure within the FAST directory. Is there any discussion of making things more modular? I can help with this, but I might have some questions as I go and it will be over the winter break in a couple of weeks. I have a student trying to use this FAST code right now, and it is not easy to get our data going.

@kexinrong
Copy link
Contributor

Thanks for your interest! Please beware that this is research code put together by a group of people overtime. The way the code is structured right now is pretty much a result of the development process. You are welcomed to ask questions and submit pull requests over the winter break if you are interested in helping with the restructuring.

For using the code, I do recommend starting with the setup in the Hector Mine example.

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

No branches or pull requests

2 participants