-
Notifications
You must be signed in to change notification settings - Fork 7
removed prop file, added cmd line, added example #21
Conversation
jpatton-USGS
commented
Mar 1, 2019
- removed use of property file(s) this depends on the corresponding neic-traveltime changes to be merged.
- added (optional) command line args for strings retrieved from property files.
- added example input and output (old style format to start)
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.
Please add an example of how to run the example input to the readme.
if(LocUtil.modelPath == null) { | ||
LocUtil.getProperties(); | ||
if (modelPath != null) { | ||
LocUtil.modelPath = modelPath; |
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.
Does this still need to be global?
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.
No, it didn't.
/** | ||
* Path for model files. | ||
*/ | ||
public static String modelPath = "./models/"; |
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.
use all caps and final for static, e.g.
public static final String DEFAULT_MODEL_PATH = "./models/";
if(LocUtil.modelPath == null) { | ||
LocUtil.getProperties(); | ||
if (modelPath != null) { | ||
modelPath = modelPath; |
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.
Reverse this check.
if (modelPath == null) {
modelPath = DEFAULT_MODEL_PATH;
}
@@ -186,6 +187,11 @@ public boolean readHydra(String eventID) { | |||
Pick pick; | |||
Pattern affinity = Pattern.compile("\\d*\\.\\d*"); | |||
|
|||
// setup event path | |||
if (eventPath != null) { | |||
LocUtil.eventPath = eventPath; |
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.
see other related changes
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.
This one is correct, if the provided event path does not equal null, then set the LocUtil.eventPath to it, otherwise let the default value in LocUtil.eventPath stand (if the provided event path is null)
// get model path | ||
String modelPath = null; | ||
if (args != null && args.length >= 1) { | ||
modelPath = args[0]; |
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.
positional arguments may not be the best, you could use
--modelPath=PATH
, then check for argument prefixes while parsing (and use defaults otherwise). Or only use argument prefixes for non-file inputs...
I'm getting compile errors:
|
There's a corresponding traveltime pr, usgs/neic-traveltime#30 that needs to be merged first. |
Running the example is still awkward, and uses a hardcoded filename. Can
|
I made the --eventFile changes already in another branch that was supposed to be the next PR after this one (https://github.com/jpatton-USGS/neic-locator/tree/loc-service). Small incremental PR's don't seem to be working out for this project, how do you suggest we do this? PR's up to the code standards pass and then one big one from there? |
Based on John's most recent comment the requested change has been made, but in a branch PR after this one, so merging this PR so the next PR's can be reviewed accordingly |