New sample: Apply mosiac rule to rasters #614

sclaridge · 2020-12-14T13:58:19Z

This sample was previously waiting for two issues to be fixed for the LOCK_RASTER method. I've set the target branch to dev so that the lock raster method will work correctly when the sample is publicly released.

Currently, the mapView is instantiated and the DrawStatusChangedListener set before the map is created so that COMPLETED draw status events will be generated and the progress indicator can be used, as advised by the Java team.

@jenmerritt I have added the new SS7 URL for the image raster service.

Thanks!

@JonLavi

Merge branch 'master' into dev

Java 11

Merge pull request master into dev (following sketch on map sample implementation)

Merge branch "master" into "dev"

Merge master into dev following FeatureCollectionLayerQuery implementation

Update README to alert Java 11 users to potential exceptions which may occur when running the project. Providing a suggested workaround from the OpenJavaFX docs.

Update README.md

JonLavi

Nice work @sclaridge , this looks great.
I've made a few suggestions for re-ordering and re-wording of comments.
One interesting thing to look at together would be the use of the member variable mosaicRule, and whether we need to set that explicitly to the imageServiceRaster every time we change it's contents.
Unfortunately I am not able to test this right now as the service seems to be down. Let's discuss it over a call and see if there's any improvements to make

JonLavi · 2020-12-15T13:27:41Z

raster/apply-mosaic-rule-to-rasters/README.md

@@ -0,0 +1,43 @@
+# Apply mosaic rule to rasters


I think it would make sense to keep raster as singular throughout, unless there's a mechanism to apply a rule to a whole group of rasters in bulk?
However if the design for this is already finalised, just disregard this comment 😁

Suggested change

# Apply mosaic rule to rasters

# Apply mosaic rule to raster

The design has been finalized, so I'll stick with rasters!

I agree with Jon here, though checking the design I now understand why it's plural:

Mosaic rules are applied to mosaic datasets, which are a mesh of several rasters managed together as one

JonLavi · 2020-12-15T13:31:46Z

.../main/java/com/esri/samples/apply_mosaic_rule_to_rasters/ApplyMosaicRuleToRastersSample.java

+      // create a combo box
+      ComboBox<String> comboBox = new ComboBox<>();
+      comboBox.setMaxWidth(Double.MAX_VALUE);


Maybe we can be a bit more specific and name this mosaicRuleComboBox?

JonLavi · 2020-12-15T13:33:34Z

.../main/java/com/esri/samples/apply_mosaic_rule_to_rasters/ApplyMosaicRuleToRastersSample.java

+      String imageServiceURL = "https://sampleserver7.arcgisonline.com/server/rest/services/amberg_germany/ImageServer";
+      ImageServiceRaster imageServiceRaster = new ImageServiceRaster(imageServiceURL);


Should we just put the URL right in the constructor to avoid making a String variable only used once?

Suggested change

String imageServiceURL = "https://sampleserver7.arcgisonline.com/server/rest/services/amberg_germany/ImageServer";

ImageServiceRaster imageServiceRaster = new ImageServiceRaster(imageServiceURL);

ImageServiceRaster imageServiceRaster =

new ImageServiceRaster("https://sampleserver7.arcgisonline.com/server/rest/services/amberg_germany/ImageServer");

JonLavi · 2020-12-15T13:36:47Z

.../main/java/com/esri/samples/apply_mosaic_rule_to_rasters/ApplyMosaicRuleToRastersSample.java

+          comboBox.getItems().addAll("Default", "Northwest", "Center", "By attribute", "Lock raster");
+
+          // set the default combo box value
+          comboBox.setValue("Default");


I've never actually seen .setValue. Should maybe double check that it's doing the same as comboBox.getSelectionModel().select(0);?

JonLavi · 2020-12-15T13:39:46Z

.../main/java/com/esri/samples/apply_mosaic_rule_to_rasters/ApplyMosaicRuleToRastersSample.java

+          // add the mosaic rules to the combo box
+          comboBox.getItems().addAll("Default", "Northwest", "Center", "By attribute", "Lock raster");
+
+          // set the default combo box value
+          comboBox.setValue("Default");


I think we can do these things independently from waiting for the RasterLayer to load, and it might make sense to move them closer to line 152 (where we add the listener: comboBox.getSelectionModel().selectedItemProperty().addListener)? That way it's clearer what these added values are going to be used for

JonLavi · 2020-12-15T14:00:01Z

.../main/java/com/esri/samples/apply_mosaic_rule_to_rasters/ApplyMosaicRuleToRastersSample.java

+      // add draw status listener to the map view
+      mapView.addDrawStatusChangedListener (drawStatusChangedEvent -> {
+        // show progress indicator while map view is drawing
+        if (drawStatusChangedEvent.getDrawStatus() == DrawStatus.IN_PROGRESS) {
+          progressIndicator.setVisible(true);
+        }
+        else {
+          progressIndicator.setVisible(false);
+        }
+      });


We can simplify this a fair bit by passing the result of the comparison == straight into .setVisible.
Up to you and @Rachael-E to decide if this is a sacrifice of legibility though

Other than that, I think the inside comment does explain the whole block, so we could get rid of // add draw status lisener...

Suggested change

// add draw status listener to the map view

mapView.addDrawStatusChangedListener (drawStatusChangedEvent -> {

// show progress indicator while map view is drawing

if (drawStatusChangedEvent.getDrawStatus() == DrawStatus.IN_PROGRESS) {

progressIndicator.setVisible(true);

}

else {

progressIndicator.setVisible(false);

}

});

// show progress indicator while map view is drawing

mapView.addDrawStatusChangedListener(drawStatusChangedEvent ->

progressIndicator.setVisible(drawStatusChangedEvent.getDrawStatus() == DrawStatus.IN_PROGRESS)

);

JonLavi · 2020-12-15T14:00:39Z

.../main/java/com/esri/samples/apply_mosaic_rule_to_rasters/ApplyMosaicRuleToRastersSample.java

+      // add raster layer as an operational layer to the map
+      map.getOperationalLayers().add(rasterLayer);
+
+      // listen for the raster layer to finish loading


I think wait is a bit more plain english than the technical term listen

Suggested change

// listen for the raster layer to finish loading

// wait for the raster layer to finish loading

JonLavi · 2020-12-15T14:01:04Z

.../main/java/com/esri/samples/apply_mosaic_rule_to_rasters/ApplyMosaicRuleToRastersSample.java

+      ImageServiceRaster imageServiceRaster = new ImageServiceRaster(imageServiceURL);
+      rasterLayer = new RasterLayer(imageServiceRaster);
+
+      // add raster layer as an operational layer to the map


Let's add an a to make it more plain english?

Suggested change

// add raster layer as an operational layer to the map

// add a raster layer as an operational layer to the map

JonLavi · 2020-12-15T14:01:31Z

.../main/java/com/esri/samples/apply_mosaic_rule_to_rasters/ApplyMosaicRuleToRastersSample.java

+      // listen for the raster layer to finish loading
+      rasterLayer.addDoneLoadingListener(() -> {
+        if (rasterLayer.getLoadStatus() == LoadStatus.LOADED) {
+          // when loaded, set map view's viewpoint to the image service raster's center


Some more possibility for plain english here as well?

Suggested change

// when loaded, set map view's viewpoint to the image service raster's center

// when loaded, set the map view's viewpoint to the image service raster's center

.../main/java/com/esri/samples/apply_mosaic_rule_to_rasters/ApplyMosaicRuleToRastersSample.java

sclaridge · 2021-02-01T10:33:17Z

@JonLavi I've created a setupUI() method to hide the hides the JavaFX controls, let me know what you think!

JonLavi

Nice work @sclaridge . Approving so that you can move on to the second review.
Just a few ideas for you.

One thing I noticed is that the progress spinner appears even when right-clicking, which can be a bit misleading as right-click doesn't do anything in this sample. It also appears when panning, which is technically correct (the MapView is in 'drawing' state), but also not really representative for what's going on.
One way to get around that would be to add the listener when making the selection in the combobox, and remove it again once the drawing is finished. If this complicates the code too much though, perhaps it would be better to leave as is.

JonLavi · 2021-02-01T13:23:26Z

.../main/java/com/esri/samples/apply_mosaic_rule_to_rasters/ApplyMosaicRuleToRastersSample.java

+    // create a combo box
+    mosaicRuleComboBox = new ComboBox<>();
+    mosaicRuleComboBox.setMaxWidth(Double.MAX_VALUE);
+
+    // add the mosaic rules to the combo box
+    mosaicRuleComboBox.getItems().addAll("Default", "Northwest", "Center", "By attribute", "Lock raster");
+
+    // set the default combo box value
+    mosaicRuleComboBox.getSelectionModel().select(0);


If you wanted, you could instantiate the combobox directly with the items, instead of adding them in a separate line,

Suggested change

// create a combo box

mosaicRuleComboBox = new ComboBox<>();

mosaicRuleComboBox.setMaxWidth(Double.MAX_VALUE);

// add the mosaic rules to the combo box

mosaicRuleComboBox.getItems().addAll("Default", "Northwest", "Center", "By attribute", "Lock raster");

// set the default combo box value

mosaicRuleComboBox.getSelectionModel().select(0);

// create a combo box with the mosaic rules

mosaicRuleComboBox = new ComboBox<>(

FXCollections.observableArrayList("Default", "Northwest", "Center", "By attribute", "Lock raster"));

mosaicRuleComboBox.setMaxWidth(Double.MAX_VALUE);

// set the default combo box value

mosaicRuleComboBox.getSelectionModel().select(0);

JonLavi · 2021-02-01T13:27:42Z

.../main/java/com/esri/samples/apply_mosaic_rule_to_rasters/ApplyMosaicRuleToRastersSample.java

@@ -0,0 +1,214 @@
+/*
+ * Copyright 2020 Esri.


2021 by now? 😁

JonLavi · 2021-02-01T13:28:35Z

.../main/java/com/esri/samples/apply_mosaic_rule_to_rasters/ApplyMosaicRuleToRastersSample.java

+  private ComboBox<String> mosaicRuleComboBox;
+  private MapView mapView;
+  private MosaicRule mosaicRule;
+  private RasterLayer rasterLayer; // keep loadable in scope to avoid garbage collection


I don't think there's any risk of RasterLayer going out of scope (it's inside the Map), but I might be wrong and misremembering the reason for this fix. Might be worth looking up again?

JonLavi · 2021-02-01T13:29:56Z

.../main/java/com/esri/samples/apply_mosaic_rule_to_rasters/ApplyMosaicRuleToRastersSample.java

+      mapView.setMap(map);
+
+      // create a raster layer from the image service raster
+      ImageServiceRaster imageServiceRaster = new ImageServiceRaster("https://sampleserver7.arcgisonline.com/server/rest/services/amberg_germany/ImageServer");


Should we stick to the character limit? I think shortening it like this would be enough

Suggested change

ImageServiceRaster imageServiceRaster = new ImageServiceRaster("https://sampleserver7.arcgisonline.com/server/rest/services/amberg_germany/ImageServer");

ImageServiceRaster imageServiceRaster = new ImageServiceRaster(

"https://sampleserver7.arcgisonline.com/server/rest/services/amberg_germany/ImageServer");

JonLavi · 2021-02-01T13:33:41Z

.../main/java/com/esri/samples/apply_mosaic_rule_to_rasters/ApplyMosaicRuleToRastersSample.java

+      setupUI();
+
+      // create a progress indicator
+      ProgressIndicator progressIndicator = new ProgressIndicator(ProgressIndicator.INDETERMINATE_PROGRESS);


Leaving out the type also works here.
Considering it's UI I'd be tempted to shove it in setupUI as well, but it would require making it a class member, so that decision is up to you!

Suggested change

ProgressIndicator progressIndicator = new ProgressIndicator(ProgressIndicator.INDETERMINATE_PROGRESS);

ProgressIndicator progressIndicator = new ProgressIndicator();

JonLavi · 2021-02-01T13:35:57Z

.../main/java/com/esri/samples/apply_mosaic_rule_to_rasters/ApplyMosaicRuleToRastersSample.java

+      // add the map view, control panel and progress indicator to the stack pane
+      stackPane.getChildren().addAll(mapView,controlsVBox, progressIndicator);
+      StackPane.setAlignment(controlsVBox, Pos.TOP_LEFT);
+      StackPane.setMargin(controlsVBox, new Insets(10, 0, 0, 10));


perhaps we can put some of this in setupUI as well?
It's good to show that the MapView belongs in the StackPane though.
It would be quite a big pattern change, so maybe worth a discussion with the team, however it'd make away with a lot of JavaFX code and leave the Runtime code front an centre, so just an idea.

JonLavi · 2021-02-01T13:36:52Z

.../main/java/com/esri/samples/apply_mosaic_rule_to_rasters/ApplyMosaicRuleToRastersSample.java

+      });
+
+      // add the map view, control panel and progress indicator to the stack pane
+      stackPane.getChildren().addAll(mapView,controlsVBox, progressIndicator);


Missing a space here:

Suggested change

stackPane.getChildren().addAll(mapView,controlsVBox, progressIndicator);

stackPane.getChildren().addAll(mapView, controlsVBox, progressIndicator);

sclaridge · 2021-02-08T17:55:47Z

@Rachael-E I've moved the JavaFX set up and progress indicator into a separate setupUI method, let me know what you think!

Rachael-E

thanks @sclaridge neat small sample! just a few minor comments and reordering of code to consider, as well as an update to the readme thanks to the new checker!

Rachael-E · 2021-02-08T19:02:46Z

raster/apply-mosaic-rule-to-rasters/build.gradle

+        // sets the API key from the gradle.properties file as a Java system property
+        systemProperty 'apiKey', apiKey


we should look into this more: if Jon is coming across this as a first time using this task others will too. I can take a look on a fresh windows setup!

Rachael-E · 2021-02-08T19:07:17Z

.../main/java/com/esri/samples/apply_mosaic_rule_to_rasters/ApplyMosaicRuleToRastersSample.java

@@ -0,0 +1,214 @@
+/*
+ * Copyright 2021 Esri.


this would have been ok to leave as 2020 since that's when the code was originally authored: no harm to update it to 2021 but just a note for future!

Rachael-E · 2021-02-09T13:34:32Z

raster/apply-mosaic-rule-to-rasters/README.md

+
+## Tags
+
+image service, mosaic rule, mosaic method, raster


I think the checker is picking up that these aren't sorted alphabetically, gosh I wouldn't have noticed this at all otherwise!

Suggested change

image service, mosaic rule, mosaic method, raster

image service, mosaic method, mosaic rule, raster

Rachael-E · 2021-02-09T13:42:03Z

raster/apply-mosaic-rule-to-rasters/README.md

@@ -0,0 +1,43 @@
+# Apply mosaic rule to rasters


I agree with Jon here, though checking the design I now understand why it's plural:

Mosaic rules are applied to mosaic datasets, which are a mesh of several rasters managed together as one

Rachael-E · 2021-02-09T13:43:01Z

.../main/java/com/esri/samples/apply_mosaic_rule_to_rasters/ApplyMosaicRuleToRastersSample.java

+      // setup the UI
+      setupUI();


let's move this to after the mapview has been set to the map: keeps consistency with our other samples where we open the code with our own Runtime specific set up.

Rachael-E · 2021-02-09T13:44:03Z

.../main/java/com/esri/samples/apply_mosaic_rule_to_rasters/ApplyMosaicRuleToRastersSample.java

+          }
+        } else {
+          // show alert if raster layer fails to load.
+          new Alert(Alert.AlertType.ERROR, "Error loading raster layer.").show();


yeh we could add that in here since this dialog box would pop up: if there is a meaningful message. Sometimes there isn't in which case we don't put that extra detail in.

Rachael-E · 2021-02-09T13:45:28Z

.../main/java/com/esri/samples/apply_mosaic_rule_to_rasters/ApplyMosaicRuleToRastersSample.java

+          case "Lock raster":
+            mosaicRule.setMosaicMethod(MosaicMethod.LOCK_RASTER);
+            mosaicRule.getLockRasterIds().clear();
+            mosaicRule.getLockRasterIds().addAll(Arrays.asList(1L, 7L, 12L));


I'm not too sure as a user why I have to add these lock raster IDs: can you add a comment to explain what this is? Is this selecting certain rasters from the mosaic dataset to display?

Rachael-E · 2021-02-09T13:50:20Z

.../main/java/com/esri/samples/apply_mosaic_rule_to_rasters/ApplyMosaicRuleToRastersSample.java

+    // create a progress indicator
+    progressIndicator = new ProgressIndicator();


I'd move the progressIndicator out of here and put it in place where it is first called in the start method. You could then also remove it as a member variable.

Rachael-E · 2021-02-09T13:51:08Z

.../main/java/com/esri/samples/apply_mosaic_rule_to_rasters/ApplyMosaicRuleToRastersSample.java

+      // show progress indicator while map view is drawing
+      mapView.addDrawStatusChangedListener(drawStatusChangedEvent ->


Suggested change

// show progress indicator while map view is drawing

mapView.addDrawStatusChangedListener(drawStatusChangedEvent ->

// show progress indicator while map view is drawing

var progressIndicator = new ProgressIndicator();

mapView.addDrawStatusChangedListener(drawStatusChangedEvent ->

sclaridge · 2021-02-10T18:27:27Z

@Rachael-E I've added back the original feature service URL and rearranged some of the UI code.

tschie and others added 30 commits October 19, 2018 15:52

update api version

8473b0c

change api version to 100.5.0

51f7c30

Merge branch 'master' into dev

6349008

Merge branch 'dev' into 100.5.0

b791ff8

change version to 100.5.0

6d1ba27

Merge branch 'master' into dev

1f322ee

Merge branch 'master' into 100.5.0

a240829

Merge branch 'master' into 100.5.0

3d6ab90

Merge branch '100.5.0' into dev

26a21ab

Merge branch 'master' into dev

e05b73e

Merge branch 'master' into dev

8f58533

start updates for java 11

75dc3a5

Merge branch 'master' into dev

e0e545c

Merge pull request #288 from Esri/master

8145835

Merge branch 'master' into dev

update java 11 parts

a8903f0

add dependencies for all platforms

82e7a94

Delete module-info.java

2ce991f

Merge branch 'dev' into java-11

d99fad3

Merge branch 'master' into dev

c8e1c13

Merge branch 'dev' into java-11

d45d1ae

Merge pull request #289 from Esri/java-11

56ed0d5

Java 11

Merge pull request #293 from Esri/master

52fb4c9

Merge pull request master into dev (following sketch on map sample implementation)

Merge branch 'master' into dev

42f678b

Merge pull request #298 from Esri/master

3a22cbb

Merge branch "master" into "dev"

Merge branch 'master' into dev

c43331e

Merge pull request #300 from Esri/master

db8110b

Merge master into dev following FeatureCollectionLayerQuery implementation

Update README

9f2204b

Update README to alert Java 11 users to potential exceptions which may occur when running the project. Providing a suggested workaround from the OpenJavaFX docs.

Update README.md

a56fbdd

Merge pull request #301 from Esri/Rachael-E-patch-1

11285fd

Update README.md

Merge branch 'master' into dev

894ed41

JonLavi reviewed Dec 15, 2020

View reviewed changes

JonLavi removed their assignment Dec 16, 2020

sclaridge added 10 commits December 16, 2020 10:59

Update ApplyMosaicRuleToRastersSample.java

7ef9ea9

Update ApplyMosaicRuleToRastersSample.java

a727c6e

Update ApplyMosaicRuleToRastersSample.java

8a24dc3

Update ApplyMosaicRuleToRastersSample.java

9535535

Update ApplyMosaicRuleToRastersSample.java

7e09406

Rename image

0c15734

Change arcgis version

0aa1a57

Re order map and map view

7130e50

Tidy map view listener

803e3a0

Add set up UI method

f234f33

sclaridge changed the base branch from dev to master February 1, 2021 10:31

Merge branch 'master' into sim11117-apply-mosiac-rule

3094a60

sclaridge requested a review from JonLavi February 1, 2021 10:39

JonLavi approved these changes Feb 1, 2021

View reviewed changes

sclaridge added 3 commits February 1, 2021 15:16

Update year

c801c07

Add back raster layer

ce4c038

Move progress indicator

134fdb3

sclaridge requested a review from Rachael-E February 8, 2021 17:55

sclaridge assigned Rachael-E Feb 8, 2021

Rachael-E reviewed Feb 9, 2021

View reviewed changes

Tidy changes from PR

a3d2423

sclaridge requested a review from Rachael-E February 10, 2021 18:27

Rachael-E approved these changes Feb 11, 2021

View reviewed changes

sclaridge merged commit c64fe50 into master Feb 12, 2021

sclaridge deleted the sim11117-apply-mosiac-rule branch February 12, 2021 18:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New sample: Apply mosiac rule to rasters #614

New sample: Apply mosiac rule to rasters #614

sclaridge commented Dec 14, 2020

JonLavi left a comment

JonLavi Dec 15, 2020

sclaridge Feb 1, 2021

Rachael-E Feb 9, 2021

JonLavi Dec 15, 2020

JonLavi Dec 15, 2020

JonLavi Dec 15, 2020

JonLavi Dec 15, 2020

JonLavi Dec 15, 2020

JonLavi Dec 15, 2020

JonLavi Dec 15, 2020

JonLavi Dec 15, 2020

sclaridge commented Feb 1, 2021

JonLavi left a comment

JonLavi Feb 1, 2021

JonLavi Feb 1, 2021

JonLavi Feb 1, 2021

JonLavi Feb 1, 2021

JonLavi Feb 1, 2021

JonLavi Feb 1, 2021

JonLavi Feb 1, 2021

sclaridge commented Feb 8, 2021

Rachael-E left a comment

Rachael-E Feb 8, 2021

Rachael-E Feb 8, 2021

Rachael-E Feb 9, 2021

Rachael-E Feb 9, 2021

Rachael-E Feb 9, 2021

Rachael-E Feb 9, 2021

Rachael-E Feb 9, 2021

Rachael-E Feb 9, 2021

Rachael-E Feb 9, 2021

sclaridge commented Feb 10, 2021

		String imageServiceURL = "https://sampleserver7.arcgisonline.com/server/rest/services/amberg_germany/ImageServer";
		ImageServiceRaster imageServiceRaster = new ImageServiceRaster(imageServiceURL);

	// listen for the raster layer to finish loading
	// wait for the raster layer to finish loading

	// add raster layer as an operational layer to the map
	// add a raster layer as an operational layer to the map

	// when loaded, set map view's viewpoint to the image service raster's center
	// when loaded, set the map view's viewpoint to the image service raster's center

	ImageServiceRaster imageServiceRaster = new ImageServiceRaster("https://sampleserver7.arcgisonline.com/server/rest/services/amberg_germany/ImageServer");
	ImageServiceRaster imageServiceRaster = new ImageServiceRaster(
	"https://sampleserver7.arcgisonline.com/server/rest/services/amberg_germany/ImageServer");

	ProgressIndicator progressIndicator = new ProgressIndicator(ProgressIndicator.INDETERMINATE_PROGRESS);
	ProgressIndicator progressIndicator = new ProgressIndicator();

	stackPane.getChildren().addAll(mapView,controlsVBox, progressIndicator);
	stackPane.getChildren().addAll(mapView, controlsVBox, progressIndicator);

		// sets the API key from the gradle.properties file as a Java system property
		systemProperty 'apiKey', apiKey

	image service, mosaic rule, mosaic method, raster
	image service, mosaic method, mosaic rule, raster

		// create a progress indicator
		progressIndicator = new ProgressIndicator();

		// show progress indicator while map view is drawing
		mapView.addDrawStatusChangedListener(drawStatusChangedEvent ->

New sample: Apply mosiac rule to rasters #614

New sample: Apply mosiac rule to rasters #614

Conversation

sclaridge commented Dec 14, 2020

JonLavi left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sclaridge commented Feb 1, 2021

JonLavi left a 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

sclaridge commented Feb 8, 2021

Rachael-E left a 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

sclaridge commented Feb 10, 2021