Improve command line arguments #632

fabiencastan · 2019-09-12T19:39:55Z

Description

This PR improves meshroom_photogrammetry and meshroom command lines.

Features list

meshroom_photogrammetry: "--save" does not disable the computation so you can save your batch reconstruction project and open it later interactively in meshroom. Additionnal --compute option if you need only to create a meshroom project file from this command line.
"meshroom -h / --help": now provides command line documentation
meshroom: new positional argument to set project file
meshroom: new "--pipeline" option to override the default pipeline with your own project file. It also supports an environment variable MESHROOM_DEFAULT_PIPELINE.
meshroom_photogrammetry: new "--paramOverrides NODE.param=value" allows to set a parameter directly from the command line.

Implementation

minor change in photogrammetry() to keep the whole graph creation in a single GraphModification.

…save, add --compute - add short names for input/output - change --save option which allow to save the graph used but now do not replace the computation - new option to disable the computation (makes sense only if you want to only save the created project)

- "meshroom -h / --help": now provides command line documentation (so argparse is done before GUI creation) - new positional argument to set project file or input folder - new "--pipeline" option to set a default pipeline, also support an environment variable

…photogrammetry pipeline

… path except if --cache is set explicitly

simogasp · 2019-09-13T08:37:26Z

meshroom/core/graph.py

+
+        Args:
+            filepath: project filepath to load
+            fileLink: Setup link to the project file, like setup cacheDir, keep filepath for save, etc.


Suggested change

fileLink: Setup link to the project file, like setup cacheDir, keep filepath for save, etc.

fileLink: if false, it only loads the project file as a template (all the data is discarded) and only the bare structure of the graph is preserved.

I found Setup link to the project file, like setup cacheDir, keep filepath for save, etc. a bit obscure/confusing

simogasp · 2019-09-13T08:39:59Z

meshroom/core/graph.py

@@ -234,7 +234,15 @@ def fileFeatures(self):
        return Graph.IO.getFeaturesForVersion(self.header.get(Graph.IO.Keys.FileVersion, "0.0"))

    @Slot(str)
-    def load(self, filepath):
+    def load(self, filepath, fileLink=True):


fileLink is a bit obscure.
why not turn the table and use asTemplate=False. I know that then all the conditions should be negate but it is less obscure than fileLink IMO.

I would prefer to avoid inverted negation: asTemplate=False will do more actions (setup the cacheDir, etc.) and asTemplate=True to do nothing.
I pushed a proposition for another name that doesn't negate the option: setupFileRef.
Does that sound fine to you?

simogasp · 2019-09-13T08:54:26Z

meshroom/ui/app.py

-            r.loadUrl(QUrl.fromLocalFile(args.project))
+            r.load(args.project)
+
+        if args.input:


shouldn't input and project be exclusive?

At least if project and input both contain a .mg. In this case, I think it would be better to throw an error to let the user know that there is something wrong. (unless it is able to load both projects?)

And if we give project and input with folders, does it add the images to the current project? Something in the same spirit of meshroom_photogrammetry https://github.com/alicevision/meshroom/pull/632/files#diff-4f70151d95f3b3a1c7ea377cbaef3f10R114

# initialize photogrammetry pipeline if args.pipeline: # custom pipeline graph = meshroom.core.graph.loadGraph(args.pipeline) cameraInit = getOnlyNodeOfType(graph, 'CameraInit') # reset graph inputs cameraInit.viewpoints.resetValue() cameraInit.intrinsics.resetValue() # add views and intrinsics (if any) read from args.input cameraInit.viewpoints.extend(views) cameraInit.intrinsics.extend(intrinsics) if not graph.canComputeLeaves: raise RuntimeError("Graph cannot be computed. Check for compatibility issues.") if args.output: publish = getOnlyNodeOfType(graph, 'Publish') publish.output.value = args.output else: # default pipeline graph = multiview.photogrammetry(inputViewpoints=views, inputIntrinsics=intrinsics, output=args.output) cameraInit = getOnlyNodeOfType(graph, 'CameraInit') if images: views, intrinsics = cameraInit.nodeDesc.buildIntrinsics(cameraInit, images) cameraInit.viewpoints.value = views cameraInit.intrinsics.value = intrinsics

Yes, there is no error for now but that's the usage intention.
So you're right, we can enforce the fact that it should be exclusive.

This is changed and I have also added another argument "--import".

- explicit error messages - allow to combine multiple inputs with new --import and --importRecursive options

simogasp · 2019-09-23T08:43:51Z

meshroom/ui/app.py

@@ -61,9 +61,13 @@ def __init__(self, args):

        parser.add_argument('input', metavar='INPUT', type=str, nargs='?',
                            help='Meshroom project file (e.g. myProject.mg) or folder with images to reconstruct.')
-        parser.add_argument('--project', metavar='MESHROOM_FILE', type=str, required=False,
+        parser.add_argument('-i', '--import', metavar='IMAGES/FOLDERS', type=str, nargs='*',


Isn't now the arguments a little bit too redundant?
We can get rid of input at this point and just use project and the imports. It makes life easier, it's more readable and less confusing, and we don't need to check for conflicts. And too bad if we break compatibility, we already broke it with photogrammetry.

simogasp · 2019-09-23T08:48:12Z

meshroom/core/graph.py

@@ -234,14 +234,14 @@ def fileFeatures(self):
        return Graph.IO.getFeaturesForVersion(self.header.get(Graph.IO.Keys.FileVersion, "0.0"))

    @Slot(str)
-    def load(self, filepath, fileLink=True):
+    def load(self, filepath, setupFileRef=True):


what about overrideProjectFile?

simogasp · 2019-09-23T08:52:44Z

meshroom/core/graph.py

+
+        Args:
+            filepath: project filepath to load
+            setupFileRef: Setup reference to the project file, like setup cacheDir, keep filepath for save, etc.


Suggested change

setupFileRef: Setup reference to the project file, like setup cacheDir, keep filepath for save, etc.

overrideProjectFile: Override the input project file (any changes will be overwritten on the file) and use its cache directories. If false, it only loads the graph of the project file as a template.

simogasp · 2019-09-25T21:02:45Z

bin/meshroom_photogrammetry

+                n.attribute(param).value = value
+        elif t == '.':
+            print('Overrides {node}.{param}={value}'.format(node=node, param=param, value=value))
+            graph.findNode(node).attribute(param).value = value


just asking, findNode(node) can fail in the same way as nodesByType(node)? Shall we manage the error?
Also, very minor thing, I'd put the print after the setting is done so it is not print in case of failure (hopefully findNode(node) or attribute(param) will print or raise some kind of error?)

In case, the same for the previous nodesByType(node)

findNode is looking for one specific node and fails with an explicit message if it doesn't find it.
nodesByType looks for all nodes of a specific type and can return an empty list without error (that's why there is a specific check to provide an error message).
I put the print before so it's more visible in the error message :s, the last NODE.PARAM is printed and then you have the error message saying that it doesn't find PARAM for instance.

fabiencastan added 5 commits September 12, 2019 21:06

[core] use logging for plugins loaded message

ad9a215

[multiview] use a single GraphModification when creating the default …

ee2fc63

…photogrammetry pipeline

[bin] meshroom_photogrammetry: setup cache folder according to --save…

979ee4b

… path except if --cache is set explicitly

fabiencastan requested a review from yann-lty September 12, 2019 19:39

fabiencastan changed the title ~~Dev/commandline args~~ Improve command line arguments Sep 12, 2019

fabiencastan requested a review from simogasp September 12, 2019 19:41

simogasp requested changes Sep 13, 2019

View reviewed changes

fabiencastan added this to the Meshroom 2019.3.0 milestone Sep 14, 2019

fabiencastan added 2 commits September 21, 2019 18:04

[ui] meshroom: improve import command line arguments

fa7b458

- explicit error messages - allow to combine multiple inputs with new --import and --importRecursive options

[core] use more explicit argument name: setupFileRef

06543f8

fabiencastan mentioned this pull request Sep 22, 2019

[question]Double graphics card CUDA... useful or not #644

Closed

natowi mentioned this pull request Sep 22, 2019

[Improvements] Project Manager/Templates #360

Open

simogasp reviewed Sep 23, 2019

View reviewed changes

fabiencastan added 3 commits September 25, 2019 21:07

[core] graph: variable rename

a53940d

[ui] simplify meshroom command line rules

ce6dcba

[bin] meshroom_photogrammetry: add command line paramOverrides

49c5789

simogasp added cli anything related to the command line type:enhancement type:pr pull request issue labels Sep 25, 2019

simogasp self-assigned this Sep 25, 2019

simogasp approved these changes Sep 25, 2019

View reviewed changes

fabiencastan added 2 commits September 26, 2019 11:35

[bin] meshroom_photogrammetry: add --inputRecursive

9a473ad

[bin] meshroom_photogrammetry: add explicit check if no image found

ab7b940

fabiencastan merged commit 7950b9f into develop Sep 26, 2019

fabiencastan deleted the dev/commandline_args branch September 26, 2019 12:42

fabiencastan mentioned this pull request Oct 22, 2019

[bin] meshroom_photogrammetry: fix argument name #682

Merged

natowi mentioned this pull request Feb 19, 2020

[request] Complete json settings file #683

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve command line arguments #632

Improve command line arguments #632

fabiencastan commented Sep 12, 2019 •

edited

Loading

simogasp Sep 13, 2019

simogasp Sep 13, 2019

fabiencastan Sep 21, 2019

simogasp Sep 13, 2019

fabiencastan Sep 13, 2019

fabiencastan Sep 21, 2019

simogasp Sep 23, 2019

simogasp Sep 23, 2019

simogasp Sep 23, 2019

simogasp Sep 25, 2019

fabiencastan Sep 26, 2019

	fileLink: Setup link to the project file, like setup cacheDir, keep filepath for save, etc.
	fileLink: if false, it only loads the project file as a template (all the data is discarded) and only the bare structure of the graph is preserved.

	setupFileRef: Setup reference to the project file, like setup cacheDir, keep filepath for save, etc.
	overrideProjectFile: Override the input project file (any changes will be overwritten on the file) and use its cache directories. If false, it only loads the graph of the project file as a template.

Improve command line arguments #632

Improve command line arguments #632

Conversation

fabiencastan commented Sep 12, 2019 • edited Loading

Description

Features list

Implementation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabiencastan commented Sep 12, 2019 •

edited

Loading