function load_input_signals and function initialize_variables

[aajayi-21]

This file will contain the subroutines used directly in the algorithm. Get_constants gets the constants that will be used in the rest of the algorithm. The parameters are from the user.

Work in progress draft, do not merge.

[sbillinge]

cool. We usually call this module parsers.py. get_constants is bad because most of these will be variables, not constants.... Actually what the functions are doing is parsing inputs so we call them something like this. It could also be called io.py and contain all the functions that handle input-output, so things that write outputs and dump to databases and whatever. For a smaller project like this maybe io.py is better?

[aajayi-21]

Ok. In this function, I would like to determine the variables used by functions in the rest of the code; some are determined directly from user input and some require only a little bit of calculation to determine from variables such as the signal length, number of components the user would like to find, and number of PDF/XRD patterns they submit. Should they all be included in this function?

Will there be a separate module that handles the command line? Also, should I include the processing of the raw PDF/XRD files in this module as well?

[sbillinge]

Ok. In this function, I would like to determine the variables used by functions in the rest of the code; some are determined directly from user input and some require only a little bit of calculation to determine from variables such as the signal length, number of components the user would like to find, and number of PDF/XRD patterns they submit. Should they all be included in this function?

Will there be a separate module that handles the command line? Also, should I include the processing of the raw PDF/XRD files in this module as well?

ah, ok, that makes sense. Maybe initializers.py? or preprocessors.py? Something like that might capture things better. Typically, parsers.py do parsing, either from the command line or from input files, i.e., they read things then assign meaning to them.

[sbillinge]

This is very good. After all our discussions, maybe keep this module as io.py as you have it and we can add any other io things there.

I would say the function name should be initialize_variables which would be more clear to me.

Otherwise it looks great.

It is probably ok to not bother writing tests for this as it doesn't do much, but if you do you will have to mock the randomizer functions.

[sbillinge]

please see comments. we already have a function for parsing these files, loaddata so we don't want to recreate it.

[sbillinge]

remove blank line here

[sbillinge]

?

[aajayi-21]

my apologies, I missed this comment. I think it should be corrected

[sbillinge]

one small question

[sbillinge]

this may be ok, but I am more used to seeing file handling being carried out in a context manager context which is considered to be more safe. There may be some nasty unwanted side-effects here, or is that handled inside loadData? Probably good to check.

[aajayi-21]

I will look into loadData and get back to you. To make sure, by 'context manager' do you mean file handling using things such as with statement or classes?

[sbillinge]

maybe google and read about python context managers?

[aajayi-21]

It seems that loadData does file handling in a context manager context.

[aajayi-21]

In the load_input_signals function, there are some behaviors I don't account for or make assumptions about. For example, when there aren't sufficient permissions to read the file/directory, when the directory contains unexpected objects(such as images), when the PDF/XRD pattern is not in the usual format, when all the files are on a unique grid, or when the directory is empty. Should I account for these issues?

[sbillinge]

In the load_input_signals function, there are some behaviors I don't account for or make assumptions about. For example, when there aren't sufficient permissions to read the file/directory, when the directory contains unexpected objects(such as images), when the PDF/XRD pattern is not in the usual format, when all the files are on a unique grid, or when the directory is empty. Should I account for these issues?

Actually, you probably do account for them because won't the code crash if it encounters them? That is how the code handles them currently. So it is not a "code handling the situation" issue as much as a "user experience/usability" issue. What experience do you want your users to have (and how much programming effort do you think it is worth putting into that)? First, check against the Use Case you are implementing and see what it has to say on this issue. Often not too much (we don't have a UC with the level of detail that it specifies that users have mixed files in their directories). But just make reasonable cost-benefit decisions here.

A guiding principle is to ask, is it better to have no program released at all because you never have time to finish all the little usability things, or to have a program that users can use starting next week, even if it is a bit hard to use. We generally prefer the latter, but then make new releases that gradually over time make it more functionally polished.

Summary:

1. Write issues for handling these things. Push off deciding in the future which to implement to increase usability. Merge the branch.... (remember have each branch do one thing)
2. Only implement it someone asks for it or you think it is quick and valuable

[sbillinge]

Nice job....code is clean and readable. Keep this up!