Skip to content
This repository has been archived by the owner on Sep 9, 2022. It is now read-only.

removed prop file, added cmd line, added example #21

Merged
merged 6 commits into from
Apr 2, 2019

Conversation

jpatton-USGS
Copy link
Collaborator

  • 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)

@jpatton-USGS jpatton-USGS requested a review from jmfee-usgs March 1, 2019 21:29
Copy link
Contributor

@jmfee-usgs jmfee-usgs left a 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;
Copy link
Contributor

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?

Copy link
Collaborator Author

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/";
Copy link
Contributor

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;
Copy link
Contributor

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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see other related changes

Copy link
Collaborator Author

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];
Copy link
Contributor

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...

@jmfee-usgs
Copy link
Contributor

I'm getting compile errors:

$ ./gradlew build

> Task :compileJava FAILED
.../neic-locator/src/main/java/gov/usgs/locator/LocMain.java:93: error: constructor TTSessionLocal in class TTSessionLocal cannot be applied to given types;
                                ttLocal = new TTSessionLocal(true, true, true, modelPath);
                                          ^
  required: boolean,boolean,boolean
  found: boolean,boolean,boolean,String
  reason: actual and formal argument lists differ in length
Note: .../neic-locator/lib/neic-traveltime/src/gov/usgs/traveltime/session/TTSession.java uses unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
1 error

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':compileJava'.
> Compilation failed; see the compiler error output for details.

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 3s
3 actionable tasks: 2 executed, 1 up-to-date

@jpatton-USGS
Copy link
Collaborator Author

There's a corresponding traveltime pr, usgs/neic-traveltime#30 that needs to be merged first.

@jmfee-usgs
Copy link
Contributor

Running the example is still awkward, and uses a hardcoded filename. Can --eventFile be made to use a specific file instead of a directory? Also, will you update the example command so it works without moving files around?

./gradlew build
cd build
java -jar libs/neic-locator-0.1.0.jar --modelPath=./models/ --eventFile=../examples/

@jpatton-USGS
Copy link
Collaborator Author

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?

@mguy-usgs mguy-usgs merged commit 635448a into usgs:master Apr 2, 2019
@mguy-usgs
Copy link
Member

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants