stretchednmfapp.py #40
Conversation
sbillinge
left a comment
sbillinge
left a comment
There was a problem hiding this comment.
bit of persnickerly cleaning suggestions.
| parser.add_argument('-i', '--input-directory', type=str, default=None, | ||
| help="Directory containing experimental data. Defaults to current working directory.") | ||
| parser.add_argument('-o', '--output-directory', type=str, | ||
| help="The directory where the results will be written. Defaults to '<input_directory/snmf_results>'.") |
There was a problem hiding this comment.
this should be <input_directory>/snmf_results i.e., replace <input_directory> with the actual input directory, but it will have a folder hanging off it which is actually called "snmf_results" by default
There was a problem hiding this comment.
I am a little confused. How should I replace <input_directory> if it's not known. Do you mean that the output_directory will always be <input_directory>/snmf_results?
There was a problem hiding this comment.
I just want you to replace <input_directory/snmf_results>, which is exactly what you wrote, with exactly <input_directory>/snmf_results.
There was a problem hiding this comment.
I understand. I will do that
| help="Directory containing experimental data. Defaults to current working directory.") | ||
| parser.add_argument('-o', '--output-directory', type=str, | ||
| help="The directory where the results will be written. Defaults to '<input_directory/snmf_results>'.") | ||
| parser.add_argument('t', '--data-type', type=str, choices=['powder_diffraction', 'pdf'], |
There was a problem hiding this comment.
when we write this we often make a global constant at the top of the file
ALLOWED_DATA_TYPES = ['powder_diffraction', 'pd', 'pair_distribution_function', 'pdf'] then use an f-string in the help that lists the allowed choices (they may list by default in which case we don't need that...you can check.)
Fixed typo in line 22, renamed 'lift' to 'lift_factor', made ALLOWED_DATA_TYPES global variable
|
this is an instruction to the user. If the user specifies input-directory
then the user will know where to find the output (hanging off that
directory). If the user doesn't specify the input directory thewill know
where to find it (hanging off current-working-directory.
…On Wed, Aug 2, 2023 at 8:01 PM Adeolu Ajayi ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In diffpy/snmf/stretchednmfapp.py
<#40 (comment)>:
> @@ -0,0 +1,26 @@
+import numpy as np
+import argparse
+from pathlib import Path
+
+
+def create_parser():
+ parser = argparse.ArgumentParser(
+ prog="stretched_nmf",
+ description="Stretched Nonnegative Matrix Factorization"
+ )
+ parser.add_argument('-i', '--input-directory', type=str, default=None,
+ help="Directory containing experimental data. Defaults to current working directory.")
+ parser.add_argument('-o', '--output-directory', type=str,
+ help="The directory where the results will be written. Defaults to '<input_directory/snmf_results>'.")
I am a little confused. How should I replace <input_directory> if it's not
known. Do you mean that the output_directory will always be
<input_directory>/snmf_results?
—
Reply to this email directly, view it on GitHub
<#40 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABAOWULLYTHUPPVDUPGUFADXTKIYZANCNFSM6AAAAAA3BK43CA>
.
You are receiving this because you commented.Message ID: <diffpy/diffpy.
***@***.***>
--
Simon Billinge
Professor, Columbia University
|
| grid, data_input = load_input_signals(args.input_directory) | ||
| lifed_data_input = lift_data(data_input, args.lift_factor) | ||
| variables = initialize_variables(lifed_data_input,args.number_of_components,data_type='pdf') | ||
| lifted_data_input = lift_data(data_input, args.lift_factor) |
There was a problem hiding this comment.
please can we change data_input to input_data everywhere (also with lifted_data_input). data_input sounds like a verb (a function name) but I find input_data clearer.
|
@aajayi-21 is this ready for merge or still a draft? |
|
It's ready to merge. The |
2 participants
stretchednmfapp.py contains the 'main' function of the software.