Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
HLA-1352: Description of JSON files #1901
HLA-1352: Description of JSON files #1901
Changes from 10 commits
f67bf8b
f674fb3
0c54cd5
967e4de
37dbbba
b49d4ac
63a509d
6005bea
891e6e5
735de0b
681cb76
352d7d4
316f052
b18916c
dfaed96
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a bit confusing below as there are sections of the configuration file, wfc3_ir_astrodrizzle_any_n1.json which I though were skipped, but they are just out of order. This needs to be corrected to avoid confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've reorganized the sections.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The discussion here seems out of context. If you are describing the contents of a configuration file, provide the name of the specific file, and then the contents. Note each numbered section maps to a corresponding section of the NameOfConfigurationFile. Perhaps this would be more obvious to me if I were looking at the formatted output. Having said this, I will look at this with a phased approach in that more and clarifying information can added later once the basics are described.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. I've added the example filename to the docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix spelling: throusholds
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change "minobj" to minimum number of objects. Fix this description regarding the type of products and the minimum number of objects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "fit quality categories" title seems out of place here.
Add a note so the user is reminded...The generate_source_catalogs section is used for alignment purposes only. This has nothing to do with the output HAP catalog products.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added them as a note now. I agree that it's not the ideal place, but I couldn't find a better place. With a "note", it seems to flow better now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you are discussing three categories at once for brevity, I would not quote the "default" values as they are not the same for all categories.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default values removed.