-
-
Notifications
You must be signed in to change notification settings - Fork 416
v.net: Add JSON output support #7088
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
base: main
Are you sure you want to change the base?
Changes from 13 commits
cbc762d
23253a4
1a45548
9894adb
1ac526b
b8ab605
ef261af
2907be7
4dd6f76
3b95796
6a837f6
4b3e8dc
d117aca
f490ab0
107dda4
35fbcf6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -9,6 +9,16 @@ void define_options(struct opt *opt) | |||||||
| { | ||||||||
| char *desc; | ||||||||
|
|
||||||||
| opt->format = G_define_standard_option(G_OPT_F_FORMAT); | ||||||||
| opt->format->key = "format"; | ||||||||
| if (!opt->format->description) | ||||||||
| opt->format->description = _("Output format"); | ||||||||
|
Comment on lines
+13
to
+15
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There should not be any need to add these.
Suggested change
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "After removing those parts and running make, the build gets stuck at the UI description generation step with the following warnings: 'Missing option key' and 'Description for option <?> missing'.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tested it and it works fine for me. Look at other usages of G_OPT_F_FORMAT.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe you'd need to clean before make, like |
||||||||
| opt->format->descriptions = _("plain;Human readable text output;" | ||||||||
| "json;JSON (JavaScript Object Notation)"); | ||||||||
| opt->format->options = "plain,json"; | ||||||||
| opt->format->answer = "plain"; | ||||||||
| opt->format->guisection = _("Print"); | ||||||||
|
|
||||||||
| opt->input = G_define_standard_option(G_OPT_V_INPUT); | ||||||||
| opt->input->required = NO; | ||||||||
| opt->input->label = _("Name of input vector line map (arcs)"); | ||||||||
|
|
@@ -123,7 +133,7 @@ void define_options(struct opt *opt) | |||||||
| } | ||||||||
|
|
||||||||
| void parse_arguments(const struct opt *opt, int *afield, int *nfield, | ||||||||
| double *thresh, int *act) | ||||||||
| double *thresh, int *act, enum OutputFormat *format) | ||||||||
| { | ||||||||
| *afield = atoi(opt->afield_opt->answer); | ||||||||
| *nfield = atoi(opt->nfield_opt->answer); | ||||||||
|
|
@@ -144,6 +154,13 @@ void parse_arguments(const struct opt *opt, int *afield, int *nfield, | |||||||
| else | ||||||||
| G_fatal_error(_("Unknown operation")); | ||||||||
|
|
||||||||
| if (strcmp(opt->format->answer, "json") == 0) { | ||||||||
| *format = FORMAT_JSON; | ||||||||
| } | ||||||||
| else { | ||||||||
| *format = FORMAT_PLAIN; | ||||||||
| } | ||||||||
|
|
||||||||
| if (*act == TOOL_NODES || *act == TOOL_CONNECT || *act == TOOL_REPORT || | ||||||||
| *act == TOOL_NREPORT || *act == TOOL_TURNTABLE) { | ||||||||
| if (opt->input->answer == NULL) | ||||||||
|
|
||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you double check which of the tests take long? In the error logs, these 6 tests in the file took 69 seconds to run on Ubuntu, which is quite long in my opinion. Maybe they were already long, but take a look to see if it is the newly added tests. About 38.4sec on macOS (the Apple silicon chips are really fast, so it's normal, about 2x as fast as the Linux runners usually). 66 sec for the Linux CMake Windows didn't finish run it yet.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've optimized the tests to address the execution time concerns. The v.net network preparation steps are now moved to setUpClass, so they only run once for the entire test suite. My local tests show a significant reduction in total runtime. I've also ensured that existing tests and the tearDown logic remain intact.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What was the before and after (approx), and what was the other tests to compare to?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I realized I was preparing the heavy streets dataset twice (once for each new test). I have now moved the network preparation to setUpClass so it only runs once. This fixed the slowdown. Before it was 38 sec. in my local mac now it is 28 sec. in my local mac. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,72 @@ | ||
| from grass.gunittest.case import TestCase | ||
| from grass.gunittest.main import test | ||
| from grass.script.core import read_command | ||
| from grass.gunittest.gmodules import SimpleModule | ||
| import json | ||
|
|
||
|
|
||
| class TestVNet(TestCase): | ||
| network = "test_vnet" | ||
|
|
||
| def tearDown(self): | ||
| """Remove viewshed map after each test method""" | ||
| # TODO: eventually, removing maps should be handled through testing framework functions | ||
| self.runModule("g.remove", flags="f", type="vector", name=self.network) | ||
|
|
||
| def test_nreport_json_output(self): | ||
| """Verify that v.net nreport produces valid and accurate JSON output""" | ||
| self.runModule( | ||
| "v.net", | ||
| input="streets", | ||
| points="schools", | ||
| output=self.network, | ||
| operation="connect", | ||
| threshold=400, | ||
| flags="c", | ||
| ) | ||
|
|
||
| vnet_module = SimpleModule( | ||
| "v.net", | ||
| input=self.network, | ||
| operation="nreport", | ||
| node_layer=2, | ||
| format="json", | ||
| ) | ||
| self.assertModule(vnet_module) | ||
|
|
||
| actual_output = json.loads(vnet_module.outputs.stdout) | ||
|
|
||
| self.assertIsInstance(actual_output, list) | ||
| self.assertGreater(len(actual_output), 0, "The JSON output list is empty") | ||
|
|
||
| expected_keys = {"node_cat", "lines"} | ||
| self.assertEqual(set(actual_output[0].keys()), expected_keys) | ||
| self.assertIsInstance(actual_output[0]["node_cat"], int) | ||
| self.assertIsInstance(actual_output[0]["lines"], list) | ||
|
|
||
| def test_report_json_output(self): | ||
| """Verify that v.net report produces valid and accurate JSON output""" | ||
| vnet_module = SimpleModule( | ||
| "v.net", | ||
| input="streets_net", | ||
|
||
| operation="report", | ||
| arc_layer=1, | ||
| node_layer=2, | ||
| format="json", | ||
| ) | ||
| self.assertModule(vnet_module) | ||
|
|
||
| actual_output = json.loads(vnet_module.outputs.stdout) | ||
|
|
||
| self.assertIsInstance(actual_output, list) | ||
| self.assertGreater(len(actual_output), 0, "The JSON output list is empty") | ||
|
|
||
| expected_keys = {"line_cat", "start_node_cat", "end_node_cat"} | ||
| self.assertEqual(set(actual_output[0].keys()), expected_keys) | ||
|
|
||
| for entry in actual_output[:10]: | ||
| for key in expected_keys: | ||
| self.assertIsInstance(entry[key], int) | ||
|
|
||
| def test_nodes(self): | ||
| """Test""" | ||
| self.assertModule( | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.