archivesunleashed / graphpass

GraphPass is a utility to filter networks and provide a default visualization output for Gephi or SigmaJS.
https://archivesunleashed.org/
Other
17 stars 2 forks source link

Implement stdin stdout procedures addressing Issue #47. #62

Closed greebie closed 6 years ago

greebie commented 6 years ago

GitHub issue(s):

47

What does this Pull Request do?

Creates a more straightforward stdin stdout procedure call.

e.g. ./graphpass /path/to/input/file /path/to/output/outfile -{flags} will run graphpass on file and store it in outfile.

The stdin and stdout paths can appear anywhere in the expression (you can put {flags} or other options before, between or after the path calls. However, the order of stdin must come before stdout.

The -i or --input will override an stdin call. The -i or --output will override a stdout call.

Both will have the same behavior as the above mentioned calls.

Graphpass will create an output/ directory in the call ./graphpass /path/to/input/file /path/to/output/outfile -{flags} but only if /path/to/ exists. Otherwise it will throw an error with a message to create /path/to/.

Any non-flag arguments beyond stdin and stdout will be ignored for now (although graphpass will store them for future use).

How should this be tested?

A description of what steps someone could take to:

Additional Notes:

As of writing, I realize that i need to update the documentation in README. I will also check the case where there is no stdout declared.

Interested parties

@ianmilligan1 and @ruebot

Thanks in advance for your help with the Archives Unleashed Project!

ruebot commented 6 years ago

Graphpass will create an output/ directory in the call

Why this? Can you explain the rationale here? It's my understanding that stdout should just be stdout. I don't understand the use case of forcing the creation of an output dir.


You also have commits in here from the previous PR.

greebie commented 6 years ago

Graphpass will create an output/ directory in the call

Why this? Can you explain the rationale here? It's my understanding that stdout should just be stdout. I don't understand the use case of forcing the creation of an output dir.

It will only try to create the output directory if stdout does not exist. Otherwise it will just use stdout.

The way the program is written now, if no stdout exists, and there is no -g flag (to switch to .gexf) the user's original file will be over-written with the graphpassed update. That's why I included code to set the output directory to ./OUT/ if stdout does not exist.

It's possible to fail on no stdout instead, but I think it should be part of a different issue, given that the unit tests work based on the current approach.

You also have commits in here from the previous PR.

I do not understand github properly. Will fix.

greebie commented 6 years ago

Getting a seg fault in Travis. Not sure why because it builds fine on my system.

ianmilligan1 commented 6 years ago

I see that it builds fine on my Mac but fails on rho

i.e.

Makefile:27: recipe for target 'install' failed
make: [install] Segmentation fault (core dumped) (ignored)
greebie commented 6 years ago

I see that it builds fine on my Mac but fails on rho

i.e.

Makefile:27: recipe for target 'install' failed
make: [install] Segmentation fault (core dumped) (ignored)

sigh That's what I thought. Going to try and force stdin stdout to see if that fixes things.

ruebot commented 6 years ago

It will only try to create the output directory if stdout does not exist. Otherwise it will just use stdout.

That's not how stdout works. We shouldn't be clobbering an existing file if an option isn't given, nor creating a new directory.

I'm going to move on to other stuff for the rest of the day, and will be at York on Thursday and Friday. So, I won't get back to reviewing this until Monday at the earliest.


I do not understand github properly. Will fix.

It's not GitHub issue. It's the process I outlined last week in Slack. Please revisit that.

greebie commented 6 years ago

Okay - the issue is that Mac has libgen.h compiled as part of gcc and Ubuntu does not. I will either add instructions to travis re: libgen.h or ignore create a function to get the directory name myself.

greebie commented 6 years ago

That that Travis is hooked up, I am going to close this PR and re-open without the confusing commit problems.