You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "vibhatha (via GitHub)" <gi...@apache.org> on 2024/03/27 11:50:46 UTC
[PR] [Java] Adding Spotless to Algorithm module [arrow]
vibhatha opened a new pull request, #40835:
URL: https://github.com/apache/arrow/pull/40835
### Rationale for this change
Adding code style and formatting options for Algorithm module.
### What changes are included in this PR?
Code formatting spotless plugin has been added.
### Are these changes tested?
Yes, but doesn't involve test cases, the plugin itself corrects.
### Are there any user-facing changes?
No
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on code in PR #40835:
URL: https://github.com/apache/arrow/pull/40835#discussion_r1542158874
##########
java/algorithm/src/main/java/org/apache/arrow/algorithm/search/VectorRangeSearcher.java:
##########
@@ -1,108 +1,108 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.arrow.algorithm.search;
-
-import org.apache.arrow.algorithm.sort.VectorValueComparator;
-import org.apache.arrow.vector.ValueVector;
-
-/**
- * Search for the range of a particular element in the target vector.
- */
-public class VectorRangeSearcher {
-
- /**
- * Result returned when a search fails.
- */
- public static final int SEARCH_FAIL_RESULT = -1;
-
- /**
- * Search for the first occurrence of an element.
- * The search is based on the binary search algorithm. So the target vector must be sorted.
- * @param targetVector the vector from which to perform the search.
- * @param comparator the criterion for the comparison.
- * @param keyVector the vector containing the element to search.
- * @param keyIndex the index of the search key in the key vector.
- * @param <V> the vector type.
- * @return the index of the first matched element if any, and -1 otherwise.
- */
- public static <V extends ValueVector> int getFirstMatch(
- V targetVector, VectorValueComparator<V> comparator, V keyVector, int keyIndex) {
- comparator.attachVectors(keyVector, targetVector);
-
- int ret = SEARCH_FAIL_RESULT;
-
- int low = 0;
- int high = targetVector.getValueCount() - 1;
-
- while (low <= high) {
- int mid = low + (high - low) / 2;
- int result = comparator.compare(keyIndex, mid);
- if (result < 0) {
- // the key is smaller
- high = mid - 1;
- } else if (result > 0) {
- // the key is larger
- low = mid + 1;
- } else {
- // an equal element is found
- // continue to go left-ward
- ret = mid;
- high = mid - 1;
- }
- }
- return ret;
- }
-
- /**
- * Search for the last occurrence of an element.
- * The search is based on the binary search algorithm. So the target vector must be sorted.
- * @param targetVector the vector from which to perform the search.
- * @param comparator the criterion for the comparison.
- * @param keyVector the vector containing the element to search.
- * @param keyIndex the index of the search key in the key vector.
- * @param <V> the vector type.
- * @return the index of the last matched element if any, and -1 otherwise.
- */
- public static <V extends ValueVector> int getLastMatch(
- V targetVector, VectorValueComparator<V> comparator, V keyVector, int keyIndex) {
- comparator.attachVectors(keyVector, targetVector);
-
- int ret = SEARCH_FAIL_RESULT;
-
- int low = 0;
- int high = targetVector.getValueCount() - 1;
-
- while (low <= high) {
- int mid = low + (high - low) / 2;
- int result = comparator.compare(keyIndex, mid);
- if (result < 0) {
- // the key is smaller
- high = mid - 1;
- } else if (result > 0) {
- // the key is larger
- low = mid + 1;
- } else {
- // an equal element is found,
- // continue to go right-ward
- ret = mid;
- low = mid + 1;
- }
- }
- return ret;
- }
-}
+/*
Review Comment:
Issue created: https://github.com/apache/arrow/issues/40854
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on code in PR #40835:
URL: https://github.com/apache/arrow/pull/40835#discussion_r1542150323
##########
java/algorithm/src/main/java/org/apache/arrow/algorithm/search/VectorRangeSearcher.java:
##########
@@ -1,108 +1,108 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.arrow.algorithm.search;
-
-import org.apache.arrow.algorithm.sort.VectorValueComparator;
-import org.apache.arrow.vector.ValueVector;
-
-/**
- * Search for the range of a particular element in the target vector.
- */
-public class VectorRangeSearcher {
-
- /**
- * Result returned when a search fails.
- */
- public static final int SEARCH_FAIL_RESULT = -1;
-
- /**
- * Search for the first occurrence of an element.
- * The search is based on the binary search algorithm. So the target vector must be sorted.
- * @param targetVector the vector from which to perform the search.
- * @param comparator the criterion for the comparison.
- * @param keyVector the vector containing the element to search.
- * @param keyIndex the index of the search key in the key vector.
- * @param <V> the vector type.
- * @return the index of the first matched element if any, and -1 otherwise.
- */
- public static <V extends ValueVector> int getFirstMatch(
- V targetVector, VectorValueComparator<V> comparator, V keyVector, int keyIndex) {
- comparator.attachVectors(keyVector, targetVector);
-
- int ret = SEARCH_FAIL_RESULT;
-
- int low = 0;
- int high = targetVector.getValueCount() - 1;
-
- while (low <= high) {
- int mid = low + (high - low) / 2;
- int result = comparator.compare(keyIndex, mid);
- if (result < 0) {
- // the key is smaller
- high = mid - 1;
- } else if (result > 0) {
- // the key is larger
- low = mid + 1;
- } else {
- // an equal element is found
- // continue to go left-ward
- ret = mid;
- high = mid - 1;
- }
- }
- return ret;
- }
-
- /**
- * Search for the last occurrence of an element.
- * The search is based on the binary search algorithm. So the target vector must be sorted.
- * @param targetVector the vector from which to perform the search.
- * @param comparator the criterion for the comparison.
- * @param keyVector the vector containing the element to search.
- * @param keyIndex the index of the search key in the key vector.
- * @param <V> the vector type.
- * @return the index of the last matched element if any, and -1 otherwise.
- */
- public static <V extends ValueVector> int getLastMatch(
- V targetVector, VectorValueComparator<V> comparator, V keyVector, int keyIndex) {
- comparator.attachVectors(keyVector, targetVector);
-
- int ret = SEARCH_FAIL_RESULT;
-
- int low = 0;
- int high = targetVector.getValueCount() - 1;
-
- while (low <= high) {
- int mid = low + (high - low) / 2;
- int result = comparator.compare(keyIndex, mid);
- if (result < 0) {
- // the key is smaller
- high = mid - 1;
- } else if (result > 0) {
- // the key is larger
- low = mid + 1;
- } else {
- // an equal element is found,
- // continue to go right-ward
- ret = mid;
- low = mid + 1;
- }
- }
- return ret;
- }
-}
+/*
Review Comment:
> \r\n -> \n
This is acceptable?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #40835:
URL: https://github.com/apache/arrow/pull/40835#discussion_r1541435146
##########
java/algorithm/src/main/java/org/apache/arrow/algorithm/search/VectorRangeSearcher.java:
##########
@@ -1,108 +1,108 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.arrow.algorithm.search;
-
-import org.apache.arrow.algorithm.sort.VectorValueComparator;
-import org.apache.arrow.vector.ValueVector;
-
-/**
- * Search for the range of a particular element in the target vector.
- */
-public class VectorRangeSearcher {
-
- /**
- * Result returned when a search fails.
- */
- public static final int SEARCH_FAIL_RESULT = -1;
-
- /**
- * Search for the first occurrence of an element.
- * The search is based on the binary search algorithm. So the target vector must be sorted.
- * @param targetVector the vector from which to perform the search.
- * @param comparator the criterion for the comparison.
- * @param keyVector the vector containing the element to search.
- * @param keyIndex the index of the search key in the key vector.
- * @param <V> the vector type.
- * @return the index of the first matched element if any, and -1 otherwise.
- */
- public static <V extends ValueVector> int getFirstMatch(
- V targetVector, VectorValueComparator<V> comparator, V keyVector, int keyIndex) {
- comparator.attachVectors(keyVector, targetVector);
-
- int ret = SEARCH_FAIL_RESULT;
-
- int low = 0;
- int high = targetVector.getValueCount() - 1;
-
- while (low <= high) {
- int mid = low + (high - low) / 2;
- int result = comparator.compare(keyIndex, mid);
- if (result < 0) {
- // the key is smaller
- high = mid - 1;
- } else if (result > 0) {
- // the key is larger
- low = mid + 1;
- } else {
- // an equal element is found
- // continue to go left-ward
- ret = mid;
- high = mid - 1;
- }
- }
- return ret;
- }
-
- /**
- * Search for the last occurrence of an element.
- * The search is based on the binary search algorithm. So the target vector must be sorted.
- * @param targetVector the vector from which to perform the search.
- * @param comparator the criterion for the comparison.
- * @param keyVector the vector containing the element to search.
- * @param keyIndex the index of the search key in the key vector.
- * @param <V> the vector type.
- * @return the index of the last matched element if any, and -1 otherwise.
- */
- public static <V extends ValueVector> int getLastMatch(
- V targetVector, VectorValueComparator<V> comparator, V keyVector, int keyIndex) {
- comparator.attachVectors(keyVector, targetVector);
-
- int ret = SEARCH_FAIL_RESULT;
-
- int low = 0;
- int high = targetVector.getValueCount() - 1;
-
- while (low <= high) {
- int mid = low + (high - low) / 2;
- int result = comparator.compare(keyIndex, mid);
- if (result < 0) {
- // the key is smaller
- high = mid - 1;
- } else if (result > 0) {
- // the key is larger
- low = mid + 1;
- } else {
- // an equal element is found,
- // continue to go right-ward
- ret = mid;
- low = mid + 1;
- }
- }
- return ret;
- }
-}
+/*
Review Comment:
\r\n -> \n
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin commented on code in PR #40835:
URL: https://github.com/apache/arrow/pull/40835#discussion_r1541363161
##########
java/algorithm/pom.xml:
##########
@@ -50,4 +55,68 @@
<build>
</build>
+
+ <profiles>
+ <profile>
+ <id>spotless-jdk11+</id>
+ <activation>
+ <jdk>[11,]</jdk>
+ </activation>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>com.diffplug.spotless</groupId>
+ <artifactId>spotless-maven-plugin</artifactId>
+ <version>${spotless.version.jdk11}</version>
+ <configuration>
+ <formats>
+ <format>
+ <!-- configure license for xml files -->
+ <includes>
+ <include>pom.xml</include>
+ </includes>
+ <licenseHeader>
+ <file>${maven.multiModuleProjectDirectory}/java/spotless/asf-xml.license</file>
+ <delimiter>(<configuration|<project)</delimiter>
+ </licenseHeader>
+ </format>
+ <format>
+ <!-- configure license for java files -->
+ <includes>
+ <include>**/*.java</include>
+ </includes>
+ <licenseHeader>
+ <file>${maven.multiModuleProjectDirectory}/java/spotless/asf-java.license</file>
+ <delimiter>package</delimiter>
+ </licenseHeader>
+</format>
+ </formats>
Review Comment:
nit: is formatting incorrect here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on PR #40835:
URL: https://github.com/apache/arrow/pull/40835#issuecomment-2022627568
@github-actions crossbow submit preview-docs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #40835:
URL: https://github.com/apache/arrow/pull/40835#issuecomment-2022582670
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
Thanks for opening a pull request!
If this is not a [minor PR](https://github.com/apache/arrow/blob/main/CONTRIBUTING.md#Minor-Fixes). Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose
Opening GitHub issues ahead of time contributes to the [Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.) of the Apache Arrow project.
Then could you also rename the pull request title in the following format?
GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
or
MINOR: [${COMPONENT}] ${SUMMARY}
In the case of PARQUET issues on JIRA the title also supports:
PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
See also:
* [Other pull requests](https://github.com/apache/arrow/pulls/)
* [Contribution Guidelines - Contributing Overview](https://arrow.apache.org/docs/developers/overview.html)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on code in PR #40835:
URL: https://github.com/apache/arrow/pull/40835#discussion_r1567786014
##########
java/algorithm/pom.xml:
##########
@@ -20,6 +20,10 @@
<name>Arrow Algorithms</name>
<description>(Experimental/Contrib) A collection of algorithms for working with ValueVectors.</description>
+ <properties>
+ <spotless.version>2.42.0</spotless.version>
Review Comment:
I can definitely try that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #40835:
URL: https://github.com/apache/arrow/pull/40835#discussion_r1542808315
##########
java/algorithm/pom.xml:
##########
@@ -50,4 +55,68 @@
<build>
</build>
+
+ <profiles>
+ <profile>
+ <id>spotless-jdk11+</id>
+ <activation>
+ <jdk>[11,]</jdk>
+ </activation>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>com.diffplug.spotless</groupId>
+ <artifactId>spotless-maven-plugin</artifactId>
+ <version>${spotless.version.jdk11}</version>
+ <configuration>
+ <formats>
+ <format>
+ <!-- configure license for xml files -->
+ <includes>
+ <include>pom.xml</include>
+ </includes>
+ <licenseHeader>
+ <file>${maven.multiModuleProjectDirectory}/java/spotless/asf-xml.license</file>
+ <delimiter>(<configuration|<project)</delimiter>
+ </licenseHeader>
+ </format>
+ <format>
+ <!-- configure license for java files -->
+ <includes>
+ <include>**/*.java</include>
+ </includes>
+ <licenseHeader>
+ <file>${maven.multiModuleProjectDirectory}/java/spotless/asf-java.license</file>
+ <delimiter>package</delimiter>
+ </licenseHeader>
+</format>
+ </formats>
Review Comment:
Yes, if there's a pre-commit plugin for XML that would be nice.
##########
docs/source/developers/java/development.rst:
##########
@@ -110,9 +110,66 @@ integration tests, you would do:
Code Style
==========
-Code style is enforced with Checkstyle. The configuration is located at `checkstyle`_.
+The current Java code styles are configured as follows:
+
+- Indent: Tabs & spaces (2 spaces per tab)
+- Google Java Format: Reformats Java source code to comply with `Google Java Style`_.
+- Configure license headers for Java & XML files
+
+Java code style is checked by `Spotless`_ during the build, and the continuous integration build will verify
+that changes adhere to the style guide.
+
+.. code-block:: xml
+
+ <java>
+ <indent>
+ ...
+ </indent>
+ <googleJavaFormat/>
+ <licenseHeader>
+ ...
+ </licenseHeader>
+ </java>
+ <pom>
+ <indent>
+ ...
+ </indent>
+ <sortPom>
+ ...
+ </sortPom>
+ </pom>
+
+Automatically fixing code style issues
+--------------------------------------
+
+- You can also just check the style without building the project with `mvn spotless:check`.
+- The Java code style can be corrected from the command line by using the following commands: `mvn spotless:apply`.
+
+.. code-block:: bash
+
+ user@machine repo % mvn spotless:check
Review Comment:
Was this copied from their docs? Can we just provide our own example or omit this?
##########
docs/source/developers/java/development.rst:
##########
@@ -110,9 +110,66 @@ integration tests, you would do:
Code Style
==========
-Code style is enforced with Checkstyle. The configuration is located at `checkstyle`_.
+The current Java code styles are configured as follows:
+
+- Indent: Tabs & spaces (2 spaces per tab)
+- Google Java Format: Reformats Java source code to comply with `Google Java Style`_.
+- Configure license headers for Java & XML files
+
+Java code style is checked by `Spotless`_ during the build, and the continuous integration build will verify
+that changes adhere to the style guide.
+
+.. code-block:: xml
Review Comment:
I don't see what this code block is for?
##########
java/algorithm/pom.xml:
##########
@@ -50,4 +55,68 @@
<build>
</build>
+
+ <profiles>
+ <profile>
+ <id>spotless-jdk11+</id>
Review Comment:
I don't want to have two separate versions of the formatter. Only build/test needs to work on JDK8.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #40835:
URL: https://github.com/apache/arrow/pull/40835#issuecomment-2022583313
:warning: GitHub issue #40819 **has been automatically assigned in GitHub** to PR creator.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #40835:
URL: https://github.com/apache/arrow/pull/40835#discussion_r1550734959
##########
java/algorithm/pom.xml:
##########
@@ -50,4 +55,68 @@
<build>
</build>
+
+ <profiles>
+ <profile>
+ <id>spotless-jdk11+</id>
+ <activation>
+ <jdk>[11,]</jdk>
+ </activation>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>com.diffplug.spotless</groupId>
+ <artifactId>spotless-maven-plugin</artifactId>
+ <version>${spotless.version.jdk11}</version>
+ <configuration>
+ <formats>
+ <format>
+ <!-- configure license for xml files -->
+ <includes>
+ <include>pom.xml</include>
+ </includes>
+ <licenseHeader>
+ <file>${maven.multiModuleProjectDirectory}/java/spotless/asf-xml.license</file>
+ <delimiter>(<configuration|<project)</delimiter>
+ </licenseHeader>
+ </format>
+ <format>
+ <!-- configure license for java files -->
+ <includes>
+ <include>**/*.java</include>
+ </includes>
+ <licenseHeader>
+ <file>${maven.multiModuleProjectDirectory}/java/spotless/asf-java.license</file>
+ <delimiter>package</delimiter>
+ </licenseHeader>
+</format>
+ </formats>
Review Comment:
We may be able to use https://github.com/prettier/plugin-xml via Prettier.
@vibhatha Could you open a new issue for this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #40835:
URL: https://github.com/apache/arrow/pull/40835#discussion_r1566564967
##########
java/algorithm/pom.xml:
##########
@@ -20,6 +20,10 @@
<name>Arrow Algorithms</name>
<description>(Experimental/Contrib) A collection of algorithms for working with ValueVectors.</description>
+ <properties>
+ <spotless.version>2.42.0</spotless.version>
Review Comment:
IMO, it's OK for this to only be Java 11+. The only thing that needs to work right now on Java 8 is build and test, not necessarily static analysis.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin commented on code in PR #40835:
URL: https://github.com/apache/arrow/pull/40835#discussion_r1541366180
##########
java/algorithm/pom.xml:
##########
@@ -50,4 +55,68 @@
<build>
</build>
+
+ <profiles>
+ <profile>
+ <id>spotless-jdk11+</id>
Review Comment:
Do we need a `spotless-jdk8`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on code in PR #40835:
URL: https://github.com/apache/arrow/pull/40835#discussion_r1542150323
##########
java/algorithm/src/main/java/org/apache/arrow/algorithm/search/VectorRangeSearcher.java:
##########
@@ -1,108 +1,108 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.arrow.algorithm.search;
-
-import org.apache.arrow.algorithm.sort.VectorValueComparator;
-import org.apache.arrow.vector.ValueVector;
-
-/**
- * Search for the range of a particular element in the target vector.
- */
-public class VectorRangeSearcher {
-
- /**
- * Result returned when a search fails.
- */
- public static final int SEARCH_FAIL_RESULT = -1;
-
- /**
- * Search for the first occurrence of an element.
- * The search is based on the binary search algorithm. So the target vector must be sorted.
- * @param targetVector the vector from which to perform the search.
- * @param comparator the criterion for the comparison.
- * @param keyVector the vector containing the element to search.
- * @param keyIndex the index of the search key in the key vector.
- * @param <V> the vector type.
- * @return the index of the first matched element if any, and -1 otherwise.
- */
- public static <V extends ValueVector> int getFirstMatch(
- V targetVector, VectorValueComparator<V> comparator, V keyVector, int keyIndex) {
- comparator.attachVectors(keyVector, targetVector);
-
- int ret = SEARCH_FAIL_RESULT;
-
- int low = 0;
- int high = targetVector.getValueCount() - 1;
-
- while (low <= high) {
- int mid = low + (high - low) / 2;
- int result = comparator.compare(keyIndex, mid);
- if (result < 0) {
- // the key is smaller
- high = mid - 1;
- } else if (result > 0) {
- // the key is larger
- low = mid + 1;
- } else {
- // an equal element is found
- // continue to go left-ward
- ret = mid;
- high = mid - 1;
- }
- }
- return ret;
- }
-
- /**
- * Search for the last occurrence of an element.
- * The search is based on the binary search algorithm. So the target vector must be sorted.
- * @param targetVector the vector from which to perform the search.
- * @param comparator the criterion for the comparison.
- * @param keyVector the vector containing the element to search.
- * @param keyIndex the index of the search key in the key vector.
- * @param <V> the vector type.
- * @return the index of the last matched element if any, and -1 otherwise.
- */
- public static <V extends ValueVector> int getLastMatch(
- V targetVector, VectorValueComparator<V> comparator, V keyVector, int keyIndex) {
- comparator.attachVectors(keyVector, targetVector);
-
- int ret = SEARCH_FAIL_RESULT;
-
- int low = 0;
- int high = targetVector.getValueCount() - 1;
-
- while (low <= high) {
- int mid = low + (high - low) / 2;
- int result = comparator.compare(keyIndex, mid);
- if (result < 0) {
- // the key is smaller
- high = mid - 1;
- } else if (result > 0) {
- // the key is larger
- low = mid + 1;
- } else {
- // an equal element is found,
- // continue to go right-ward
- ret = mid;
- low = mid + 1;
- }
- }
- return ret;
- }
-}
+/*
Review Comment:
> \r\n -> \n
This is acceptable?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #40835:
URL: https://github.com/apache/arrow/pull/40835#discussion_r1542181268
##########
java/algorithm/pom.xml:
##########
@@ -50,4 +55,68 @@
<build>
</build>
+
+ <profiles>
+ <profile>
+ <id>spotless-jdk11+</id>
+ <activation>
+ <jdk>[11,]</jdk>
+ </activation>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>com.diffplug.spotless</groupId>
+ <artifactId>spotless-maven-plugin</artifactId>
+ <version>${spotless.version.jdk11}</version>
+ <configuration>
+ <formats>
+ <format>
+ <!-- configure license for xml files -->
+ <includes>
+ <include>pom.xml</include>
+ </includes>
+ <licenseHeader>
+ <file>${maven.multiModuleProjectDirectory}/java/spotless/asf-xml.license</file>
+ <delimiter>(<configuration|<project)</delimiter>
+ </licenseHeader>
+ </format>
+ <format>
+ <!-- configure license for java files -->
+ <includes>
+ <include>**/*.java</include>
+ </includes>
+ <licenseHeader>
+ <file>${maven.multiModuleProjectDirectory}/java/spotless/asf-java.license</file>
+ <delimiter>package</delimiter>
+ </licenseHeader>
+</format>
+ </formats>
Review Comment:
Should we add a formatter for XML too? :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "laurentgo (via GitHub)" <gi...@apache.org>.
laurentgo commented on code in PR #40835:
URL: https://github.com/apache/arrow/pull/40835#discussion_r1566628696
##########
java/algorithm/pom.xml:
##########
@@ -20,6 +20,10 @@
<name>Arrow Algorithms</name>
<description>(Experimental/Contrib) A collection of algorithms for working with ValueVectors.</description>
+ <properties>
+ <spotless.version>2.42.0</spotless.version>
Review Comment:
For people still using Java 8 to build Arrow, it means they may not notice a formatting issue (or apply a fix for it) unless they submit a pull request or use a different java version. Workable but not ideal
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "laurentgo (via GitHub)" <gi...@apache.org>.
laurentgo commented on code in PR #40835:
URL: https://github.com/apache/arrow/pull/40835#discussion_r1566025123
##########
java/algorithm/pom.xml:
##########
@@ -50,4 +54,74 @@
<build>
</build>
+
+ <profiles>
+ <profile>
+ <id>spotless</id>
+ <activation>
+ <activeByDefault>true</activeByDefault>
Review Comment:
Why is it being added to a profile? My personal experience with `activeByDefault` is that people are not always familiar with how it works, notably that one need to manually activate the profile if another profile is being activated with `-P` option.
##########
java/algorithm/pom.xml:
##########
@@ -50,4 +54,74 @@
<build>
</build>
+
+ <profiles>
+ <profile>
+ <id>spotless</id>
+ <activation>
+ <activeByDefault>true</activeByDefault>
+ </activation>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>com.diffplug.spotless</groupId>
+ <artifactId>spotless-maven-plugin</artifactId>
+ <version>${spotless.version}</version>
Review Comment:
Shoudn't this be managed at the top level pom.xml in `pluginManagement` if we are planning to enable it for all modules?
##########
java/algorithm/pom.xml:
##########
@@ -20,6 +20,10 @@
<name>Arrow Algorithms</name>
<description>(Experimental/Contrib) A collection of algorithms for working with ValueVectors.</description>
+ <properties>
+ <spotless.version>2.42.0</spotless.version>
Review Comment:
This version does not work with Java 8
##########
java/algorithm/src/main/java/org/apache/arrow/algorithm/search/VectorRangeSearcher.java:
##########
@@ -1,108 +1,108 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.arrow.algorithm.search;
-
-import org.apache.arrow.algorithm.sort.VectorValueComparator;
-import org.apache.arrow.vector.ValueVector;
-
-/**
- * Search for the range of a particular element in the target vector.
- */
-public class VectorRangeSearcher {
-
- /**
- * Result returned when a search fails.
- */
- public static final int SEARCH_FAIL_RESULT = -1;
-
- /**
- * Search for the first occurrence of an element.
- * The search is based on the binary search algorithm. So the target vector must be sorted.
- * @param targetVector the vector from which to perform the search.
- * @param comparator the criterion for the comparison.
- * @param keyVector the vector containing the element to search.
- * @param keyIndex the index of the search key in the key vector.
- * @param <V> the vector type.
- * @return the index of the first matched element if any, and -1 otherwise.
- */
- public static <V extends ValueVector> int getFirstMatch(
- V targetVector, VectorValueComparator<V> comparator, V keyVector, int keyIndex) {
- comparator.attachVectors(keyVector, targetVector);
-
- int ret = SEARCH_FAIL_RESULT;
-
- int low = 0;
- int high = targetVector.getValueCount() - 1;
-
- while (low <= high) {
- int mid = low + (high - low) / 2;
- int result = comparator.compare(keyIndex, mid);
- if (result < 0) {
- // the key is smaller
- high = mid - 1;
- } else if (result > 0) {
- // the key is larger
- low = mid + 1;
- } else {
- // an equal element is found
- // continue to go left-ward
- ret = mid;
- high = mid - 1;
- }
- }
- return ret;
- }
-
- /**
- * Search for the last occurrence of an element.
- * The search is based on the binary search algorithm. So the target vector must be sorted.
- * @param targetVector the vector from which to perform the search.
- * @param comparator the criterion for the comparison.
- * @param keyVector the vector containing the element to search.
- * @param keyIndex the index of the search key in the key vector.
- * @param <V> the vector type.
- * @return the index of the last matched element if any, and -1 otherwise.
- */
- public static <V extends ValueVector> int getLastMatch(
- V targetVector, VectorValueComparator<V> comparator, V keyVector, int keyIndex) {
- comparator.attachVectors(keyVector, targetVector);
-
- int ret = SEARCH_FAIL_RESULT;
-
- int low = 0;
- int high = targetVector.getValueCount() - 1;
-
- while (low <= high) {
- int mid = low + (high - low) / 2;
- int result = comparator.compare(keyIndex, mid);
- if (result < 0) {
- // the key is smaller
- high = mid - 1;
- } else if (result > 0) {
- // the key is larger
- low = mid + 1;
- } else {
- // an equal element is found,
- // continue to go right-ward
- ret = mid;
- low = mid + 1;
- }
- }
- return ret;
- }
-}
+/*
Review Comment:
The issue with this file are the line terminators (this is a Windows text file basically)
##########
java/algorithm/pom.xml:
##########
@@ -50,4 +54,74 @@
<build>
</build>
+
+ <profiles>
+ <profile>
+ <id>spotless</id>
+ <activation>
+ <activeByDefault>true</activeByDefault>
+ </activation>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>com.diffplug.spotless</groupId>
+ <artifactId>spotless-maven-plugin</artifactId>
+ <version>${spotless.version}</version>
+ <configuration>
+ <formats>
+ <format>
+ <!-- configure license for xml files -->
+ <includes>
+ <include>pom.xml</include>
+ </includes>
+ <licenseHeader>
+ <file>${maven.multiModuleProjectDirectory}/java/spotless/asf-xml.license</file>
+ <delimiter>(<configuration|<project)</delimiter>
+ </licenseHeader>
+ </format>
+ <format>
+ <!-- configure license for java files -->
+ <includes>
+ <include>**/*.java</include>
+ </includes>
+ <licenseHeader>
+ <file>${maven.multiModuleProjectDirectory}/java/spotless/asf-java.license</file>
+ <delimiter>package</delimiter>
+ </licenseHeader>
+ </format>
+ </formats>
+ <java>
+ <googleJavaFormat>
+ <version>1.9</version>
Review Comment:
`1.9` is quite old but also is not compatible with Java 8 (I think the last Java 8 version is `1.7`)
##########
java/algorithm/pom.xml:
##########
@@ -50,4 +54,74 @@
<build>
</build>
+
+ <profiles>
+ <profile>
+ <id>spotless</id>
+ <activation>
+ <activeByDefault>true</activeByDefault>
+ </activation>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>com.diffplug.spotless</groupId>
+ <artifactId>spotless-maven-plugin</artifactId>
+ <version>${spotless.version}</version>
+ <configuration>
+ <formats>
+ <format>
+ <!-- configure license for xml files -->
+ <includes>
+ <include>pom.xml</include>
+ </includes>
+ <licenseHeader>
+ <file>${maven.multiModuleProjectDirectory}/java/spotless/asf-xml.license</file>
+ <delimiter>(<configuration|<project)</delimiter>
+ </licenseHeader>
+ </format>
+ <format>
+ <!-- configure license for java files -->
+ <includes>
+ <include>**/*.java</include>
+ </includes>
+ <licenseHeader>
+ <file>${maven.multiModuleProjectDirectory}/java/spotless/asf-java.license</file>
+ <delimiter>package</delimiter>
+ </licenseHeader>
+ </format>
+ </formats>
+ <java>
+ <googleJavaFormat>
+ <version>1.9</version>
+ <style>GOOGLE</style>
+ </googleJavaFormat>
+ </java>
+ <pom>
+ <indent>
+ <tabs>true</tabs>
Review Comment:
Not sure why having a rule to use tab indentation + a rule for space indentation + sortPom (which also handles indentation with default being 2 space per indentation level). Isn't `sortPom` enough?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #40835:
URL: https://github.com/apache/arrow/pull/40835#discussion_r1568028738
##########
java/algorithm/pom.xml:
##########
@@ -50,4 +54,74 @@
<build>
</build>
+
+ <profiles>
+ <profile>
+ <id>spotless</id>
+ <activation>
+ <activeByDefault>true</activeByDefault>
+ </activation>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>com.diffplug.spotless</groupId>
+ <artifactId>spotless-maven-plugin</artifactId>
+ <version>${spotless.version}</version>
+ <configuration>
+ <formats>
+ <format>
+ <!-- configure license for xml files -->
+ <includes>
+ <include>pom.xml</include>
+ </includes>
+ <licenseHeader>
+ <file>${maven.multiModuleProjectDirectory}/java/spotless/asf-xml.license</file>
+ <delimiter>(<configuration|<project)</delimiter>
+ </licenseHeader>
+ </format>
+ <format>
+ <!-- configure license for java files -->
+ <includes>
+ <include>**/*.java</include>
+ </includes>
+ <licenseHeader>
+ <file>${maven.multiModuleProjectDirectory}/java/spotless/asf-java.license</file>
+ <delimiter>package</delimiter>
+ </licenseHeader>
+ </format>
+ </formats>
+ <java>
+ <googleJavaFormat>
+ <version>1.9</version>
Review Comment:
Well, which is it? Either both should be compatible or neither should be compatible. If you are going to use an older version of Spotless then also use an older version here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #40835:
URL: https://github.com/apache/arrow/pull/40835#issuecomment-2022631773
Revision: f5a843dd6d36d2c5a387f55de9a1fe275559bad7
Submitted crossbow builds: [ursacomputing/crossbow @ actions-b633603bed](https://github.com/ursacomputing/crossbow/branches/all?query=actions-b633603bed)
|Task|Status|
|----|------|
|preview-docs|[](https://github.com/ursacomputing/crossbow/actions/runs/8451601460/job/23150275730)|
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin commented on code in PR #40835:
URL: https://github.com/apache/arrow/pull/40835#discussion_r1541357298
##########
java/algorithm/src/main/java/org/apache/arrow/algorithm/search/VectorRangeSearcher.java:
##########
@@ -1,108 +1,108 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.arrow.algorithm.search;
-
-import org.apache.arrow.algorithm.sort.VectorValueComparator;
-import org.apache.arrow.vector.ValueVector;
-
-/**
- * Search for the range of a particular element in the target vector.
- */
-public class VectorRangeSearcher {
-
- /**
- * Result returned when a search fails.
- */
- public static final int SEARCH_FAIL_RESULT = -1;
-
- /**
- * Search for the first occurrence of an element.
- * The search is based on the binary search algorithm. So the target vector must be sorted.
- * @param targetVector the vector from which to perform the search.
- * @param comparator the criterion for the comparison.
- * @param keyVector the vector containing the element to search.
- * @param keyIndex the index of the search key in the key vector.
- * @param <V> the vector type.
- * @return the index of the first matched element if any, and -1 otherwise.
- */
- public static <V extends ValueVector> int getFirstMatch(
- V targetVector, VectorValueComparator<V> comparator, V keyVector, int keyIndex) {
- comparator.attachVectors(keyVector, targetVector);
-
- int ret = SEARCH_FAIL_RESULT;
-
- int low = 0;
- int high = targetVector.getValueCount() - 1;
-
- while (low <= high) {
- int mid = low + (high - low) / 2;
- int result = comparator.compare(keyIndex, mid);
- if (result < 0) {
- // the key is smaller
- high = mid - 1;
- } else if (result > 0) {
- // the key is larger
- low = mid + 1;
- } else {
- // an equal element is found
- // continue to go left-ward
- ret = mid;
- high = mid - 1;
- }
- }
- return ret;
- }
-
- /**
- * Search for the last occurrence of an element.
- * The search is based on the binary search algorithm. So the target vector must be sorted.
- * @param targetVector the vector from which to perform the search.
- * @param comparator the criterion for the comparison.
- * @param keyVector the vector containing the element to search.
- * @param keyIndex the index of the search key in the key vector.
- * @param <V> the vector type.
- * @return the index of the last matched element if any, and -1 otherwise.
- */
- public static <V extends ValueVector> int getLastMatch(
- V targetVector, VectorValueComparator<V> comparator, V keyVector, int keyIndex) {
- comparator.attachVectors(keyVector, targetVector);
-
- int ret = SEARCH_FAIL_RESULT;
-
- int low = 0;
- int high = targetVector.getValueCount() - 1;
-
- while (low <= high) {
- int mid = low + (high - low) / 2;
- int result = comparator.compare(keyIndex, mid);
- if (result < 0) {
- // the key is smaller
- high = mid - 1;
- } else if (result > 0) {
- // the key is larger
- low = mid + 1;
- } else {
- // an equal element is found,
- // continue to go right-ward
- ret = mid;
- low = mid + 1;
- }
- }
- return ret;
- }
-}
+/*
Review Comment:
Interesting! Optional, but what happens if you diff the before/after files? Overall, I think its fine.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on code in PR #40835:
URL: https://github.com/apache/arrow/pull/40835#discussion_r1549385351
##########
docs/source/developers/java/development.rst:
##########
@@ -110,9 +110,66 @@ integration tests, you would do:
Code Style
==========
-Code style is enforced with Checkstyle. The configuration is located at `checkstyle`_.
+The current Java code styles are configured as follows:
+
+- Indent: Tabs & spaces (2 spaces per tab)
+- Google Java Format: Reformats Java source code to comply with `Google Java Style`_.
+- Configure license headers for Java & XML files
+
+Java code style is checked by `Spotless`_ during the build, and the continuous integration build will verify
+that changes adhere to the style guide.
+
+.. code-block:: xml
+
+ <java>
+ <indent>
+ ...
+ </indent>
+ <googleJavaFormat/>
+ <licenseHeader>
+ ...
+ </licenseHeader>
+ </java>
+ <pom>
+ <indent>
+ ...
+ </indent>
+ <sortPom>
+ ...
+ </sortPom>
+ </pom>
+
+Automatically fixing code style issues
+--------------------------------------
+
+- You can also just check the style without building the project with `mvn spotless:check`.
+- The Java code style can be corrected from the command line by using the following commands: `mvn spotless:apply`.
+
+.. code-block:: bash
+
+ user@machine repo % mvn spotless:check
Review Comment:
I adopted some of the things from the older PR. Let me verify. Still an early draft. I will clean up before marking this ready for review. Sorry about this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on code in PR #40835:
URL: https://github.com/apache/arrow/pull/40835#discussion_r1541415256
##########
java/algorithm/src/main/java/org/apache/arrow/algorithm/search/VectorRangeSearcher.java:
##########
@@ -1,108 +1,108 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.arrow.algorithm.search;
-
-import org.apache.arrow.algorithm.sort.VectorValueComparator;
-import org.apache.arrow.vector.ValueVector;
-
-/**
- * Search for the range of a particular element in the target vector.
- */
-public class VectorRangeSearcher {
-
- /**
- * Result returned when a search fails.
- */
- public static final int SEARCH_FAIL_RESULT = -1;
-
- /**
- * Search for the first occurrence of an element.
- * The search is based on the binary search algorithm. So the target vector must be sorted.
- * @param targetVector the vector from which to perform the search.
- * @param comparator the criterion for the comparison.
- * @param keyVector the vector containing the element to search.
- * @param keyIndex the index of the search key in the key vector.
- * @param <V> the vector type.
- * @return the index of the first matched element if any, and -1 otherwise.
- */
- public static <V extends ValueVector> int getFirstMatch(
- V targetVector, VectorValueComparator<V> comparator, V keyVector, int keyIndex) {
- comparator.attachVectors(keyVector, targetVector);
-
- int ret = SEARCH_FAIL_RESULT;
-
- int low = 0;
- int high = targetVector.getValueCount() - 1;
-
- while (low <= high) {
- int mid = low + (high - low) / 2;
- int result = comparator.compare(keyIndex, mid);
- if (result < 0) {
- // the key is smaller
- high = mid - 1;
- } else if (result > 0) {
- // the key is larger
- low = mid + 1;
- } else {
- // an equal element is found
- // continue to go left-ward
- ret = mid;
- high = mid - 1;
- }
- }
- return ret;
- }
-
- /**
- * Search for the last occurrence of an element.
- * The search is based on the binary search algorithm. So the target vector must be sorted.
- * @param targetVector the vector from which to perform the search.
- * @param comparator the criterion for the comparison.
- * @param keyVector the vector containing the element to search.
- * @param keyIndex the index of the search key in the key vector.
- * @param <V> the vector type.
- * @return the index of the last matched element if any, and -1 otherwise.
- */
- public static <V extends ValueVector> int getLastMatch(
- V targetVector, VectorValueComparator<V> comparator, V keyVector, int keyIndex) {
- comparator.attachVectors(keyVector, targetVector);
-
- int ret = SEARCH_FAIL_RESULT;
-
- int low = 0;
- int high = targetVector.getValueCount() - 1;
-
- while (low <= high) {
- int mid = low + (high - low) / 2;
- int result = comparator.compare(keyIndex, mid);
- if (result < 0) {
- // the key is smaller
- high = mid - 1;
- } else if (result > 0) {
- // the key is larger
- low = mid + 1;
- } else {
- // an equal element is found,
- // continue to go right-ward
- ret = mid;
- low = mid + 1;
- }
- }
- return ret;
- }
-}
+/*
Review Comment:
Good point, I did compare it with the main branch, it shows no change. That's the odd part. I need to check this again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on code in PR #40835:
URL: https://github.com/apache/arrow/pull/40835#discussion_r1542154916
##########
java/algorithm/pom.xml:
##########
@@ -50,4 +55,68 @@
<build>
</build>
+
+ <profiles>
+ <profile>
+ <id>spotless-jdk11+</id>
Review Comment:
I assume we still do have some JDK 8 developers? That's why I wanted to leave it here, but if it is not a very strong case, we can definitely remove it. WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on code in PR #40835:
URL: https://github.com/apache/arrow/pull/40835#discussion_r1567776822
##########
java/algorithm/pom.xml:
##########
@@ -50,4 +54,74 @@
<build>
</build>
+
+ <profiles>
+ <profile>
+ <id>spotless</id>
+ <activation>
+ <activeByDefault>true</activeByDefault>
+ </activation>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>com.diffplug.spotless</groupId>
+ <artifactId>spotless-maven-plugin</artifactId>
+ <version>${spotless.version}</version>
+ <configuration>
+ <formats>
+ <format>
+ <!-- configure license for xml files -->
+ <includes>
+ <include>pom.xml</include>
+ </includes>
+ <licenseHeader>
+ <file>${maven.multiModuleProjectDirectory}/java/spotless/asf-xml.license</file>
+ <delimiter>(<configuration|<project)</delimiter>
+ </licenseHeader>
+ </format>
+ <format>
+ <!-- configure license for java files -->
+ <includes>
+ <include>**/*.java</include>
+ </includes>
+ <licenseHeader>
+ <file>${maven.multiModuleProjectDirectory}/java/spotless/asf-java.license</file>
+ <delimiter>package</delimiter>
+ </licenseHeader>
+ </format>
+ </formats>
+ <java>
+ <googleJavaFormat>
+ <version>1.9</version>
Review Comment:
I skipped Java 8 for this change, but we can re-think about this. cc @lidavidm
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on code in PR #40835:
URL: https://github.com/apache/arrow/pull/40835#discussion_r1567776306
##########
java/algorithm/pom.xml:
##########
@@ -20,6 +20,10 @@
<name>Arrow Algorithms</name>
<description>(Experimental/Contrib) A collection of algorithms for working with ValueVectors.</description>
+ <properties>
+ <spotless.version>2.42.0</spotless.version>
Review Comment:
Earlier I added a separate version for Java 8, but we settled to keep just one. @lidavidm should we add it back?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #40835:
URL: https://github.com/apache/arrow/pull/40835#discussion_r1541436698
##########
java/algorithm/src/main/java/org/apache/arrow/algorithm/search/VectorRangeSearcher.java:
##########
@@ -1,108 +1,108 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.arrow.algorithm.search;
-
-import org.apache.arrow.algorithm.sort.VectorValueComparator;
-import org.apache.arrow.vector.ValueVector;
-
-/**
- * Search for the range of a particular element in the target vector.
- */
-public class VectorRangeSearcher {
-
- /**
- * Result returned when a search fails.
- */
- public static final int SEARCH_FAIL_RESULT = -1;
-
- /**
- * Search for the first occurrence of an element.
- * The search is based on the binary search algorithm. So the target vector must be sorted.
- * @param targetVector the vector from which to perform the search.
- * @param comparator the criterion for the comparison.
- * @param keyVector the vector containing the element to search.
- * @param keyIndex the index of the search key in the key vector.
- * @param <V> the vector type.
- * @return the index of the first matched element if any, and -1 otherwise.
- */
- public static <V extends ValueVector> int getFirstMatch(
- V targetVector, VectorValueComparator<V> comparator, V keyVector, int keyIndex) {
- comparator.attachVectors(keyVector, targetVector);
-
- int ret = SEARCH_FAIL_RESULT;
-
- int low = 0;
- int high = targetVector.getValueCount() - 1;
-
- while (low <= high) {
- int mid = low + (high - low) / 2;
- int result = comparator.compare(keyIndex, mid);
- if (result < 0) {
- // the key is smaller
- high = mid - 1;
- } else if (result > 0) {
- // the key is larger
- low = mid + 1;
- } else {
- // an equal element is found
- // continue to go left-ward
- ret = mid;
- high = mid - 1;
- }
- }
- return ret;
- }
-
- /**
- * Search for the last occurrence of an element.
- * The search is based on the binary search algorithm. So the target vector must be sorted.
- * @param targetVector the vector from which to perform the search.
- * @param comparator the criterion for the comparison.
- * @param keyVector the vector containing the element to search.
- * @param keyIndex the index of the search key in the key vector.
- * @param <V> the vector type.
- * @return the index of the last matched element if any, and -1 otherwise.
- */
- public static <V extends ValueVector> int getLastMatch(
- V targetVector, VectorValueComparator<V> comparator, V keyVector, int keyIndex) {
- comparator.attachVectors(keyVector, targetVector);
-
- int ret = SEARCH_FAIL_RESULT;
-
- int low = 0;
- int high = targetVector.getValueCount() - 1;
-
- while (low <= high) {
- int mid = low + (high - low) / 2;
- int result = comparator.compare(keyIndex, mid);
- if (result < 0) {
- // the key is smaller
- high = mid - 1;
- } else if (result > 0) {
- // the key is larger
- low = mid + 1;
- } else {
- // an equal element is found,
- // continue to go right-ward
- ret = mid;
- low = mid + 1;
- }
- }
- return ret;
- }
-}
+/*
Review Comment:
@kou is exploring pre-commit (https://github.com/apache/arrow/issues/40417) which has checks for this sort of thing. Would Spotless be able to integrate with pre-commit?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on code in PR #40835:
URL: https://github.com/apache/arrow/pull/40835#discussion_r1542152553
##########
java/algorithm/src/main/java/org/apache/arrow/algorithm/search/VectorRangeSearcher.java:
##########
@@ -1,108 +1,108 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.arrow.algorithm.search;
-
-import org.apache.arrow.algorithm.sort.VectorValueComparator;
-import org.apache.arrow.vector.ValueVector;
-
-/**
- * Search for the range of a particular element in the target vector.
- */
-public class VectorRangeSearcher {
-
- /**
- * Result returned when a search fails.
- */
- public static final int SEARCH_FAIL_RESULT = -1;
-
- /**
- * Search for the first occurrence of an element.
- * The search is based on the binary search algorithm. So the target vector must be sorted.
- * @param targetVector the vector from which to perform the search.
- * @param comparator the criterion for the comparison.
- * @param keyVector the vector containing the element to search.
- * @param keyIndex the index of the search key in the key vector.
- * @param <V> the vector type.
- * @return the index of the first matched element if any, and -1 otherwise.
- */
- public static <V extends ValueVector> int getFirstMatch(
- V targetVector, VectorValueComparator<V> comparator, V keyVector, int keyIndex) {
- comparator.attachVectors(keyVector, targetVector);
-
- int ret = SEARCH_FAIL_RESULT;
-
- int low = 0;
- int high = targetVector.getValueCount() - 1;
-
- while (low <= high) {
- int mid = low + (high - low) / 2;
- int result = comparator.compare(keyIndex, mid);
- if (result < 0) {
- // the key is smaller
- high = mid - 1;
- } else if (result > 0) {
- // the key is larger
- low = mid + 1;
- } else {
- // an equal element is found
- // continue to go left-ward
- ret = mid;
- high = mid - 1;
- }
- }
- return ret;
- }
-
- /**
- * Search for the last occurrence of an element.
- * The search is based on the binary search algorithm. So the target vector must be sorted.
- * @param targetVector the vector from which to perform the search.
- * @param comparator the criterion for the comparison.
- * @param keyVector the vector containing the element to search.
- * @param keyIndex the index of the search key in the key vector.
- * @param <V> the vector type.
- * @return the index of the last matched element if any, and -1 otherwise.
- */
- public static <V extends ValueVector> int getLastMatch(
- V targetVector, VectorValueComparator<V> comparator, V keyVector, int keyIndex) {
- comparator.attachVectors(keyVector, targetVector);
-
- int ret = SEARCH_FAIL_RESULT;
-
- int low = 0;
- int high = targetVector.getValueCount() - 1;
-
- while (low <= high) {
- int mid = low + (high - low) / 2;
- int result = comparator.compare(keyIndex, mid);
- if (result < 0) {
- // the key is smaller
- high = mid - 1;
- } else if (result > 0) {
- // the key is larger
- low = mid + 1;
- } else {
- // an equal element is found,
- // continue to go right-ward
- ret = mid;
- low = mid + 1;
- }
- }
- return ret;
- }
-}
+/*
Review Comment:
@lidavidm I think we should be able to integrate it. We could create a bash script which does `mvn spotless:apply` and format the code. Shall we investigate in a separate PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on code in PR #40835:
URL: https://github.com/apache/arrow/pull/40835#discussion_r1540994541
##########
java/algorithm/src/main/java/org/apache/arrow/algorithm/search/VectorRangeSearcher.java:
##########
@@ -1,108 +1,108 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.arrow.algorithm.search;
-
-import org.apache.arrow.algorithm.sort.VectorValueComparator;
-import org.apache.arrow.vector.ValueVector;
-
-/**
- * Search for the range of a particular element in the target vector.
- */
-public class VectorRangeSearcher {
-
- /**
- * Result returned when a search fails.
- */
- public static final int SEARCH_FAIL_RESULT = -1;
-
- /**
- * Search for the first occurrence of an element.
- * The search is based on the binary search algorithm. So the target vector must be sorted.
- * @param targetVector the vector from which to perform the search.
- * @param comparator the criterion for the comparison.
- * @param keyVector the vector containing the element to search.
- * @param keyIndex the index of the search key in the key vector.
- * @param <V> the vector type.
- * @return the index of the first matched element if any, and -1 otherwise.
- */
- public static <V extends ValueVector> int getFirstMatch(
- V targetVector, VectorValueComparator<V> comparator, V keyVector, int keyIndex) {
- comparator.attachVectors(keyVector, targetVector);
-
- int ret = SEARCH_FAIL_RESULT;
-
- int low = 0;
- int high = targetVector.getValueCount() - 1;
-
- while (low <= high) {
- int mid = low + (high - low) / 2;
- int result = comparator.compare(keyIndex, mid);
- if (result < 0) {
- // the key is smaller
- high = mid - 1;
- } else if (result > 0) {
- // the key is larger
- low = mid + 1;
- } else {
- // an equal element is found
- // continue to go left-ward
- ret = mid;
- high = mid - 1;
- }
- }
- return ret;
- }
-
- /**
- * Search for the last occurrence of an element.
- * The search is based on the binary search algorithm. So the target vector must be sorted.
- * @param targetVector the vector from which to perform the search.
- * @param comparator the criterion for the comparison.
- * @param keyVector the vector containing the element to search.
- * @param keyIndex the index of the search key in the key vector.
- * @param <V> the vector type.
- * @return the index of the last matched element if any, and -1 otherwise.
- */
- public static <V extends ValueVector> int getLastMatch(
- V targetVector, VectorValueComparator<V> comparator, V keyVector, int keyIndex) {
- comparator.attachVectors(keyVector, targetVector);
-
- int ret = SEARCH_FAIL_RESULT;
-
- int low = 0;
- int high = targetVector.getValueCount() - 1;
-
- while (low <= high) {
- int mid = low + (high - low) / 2;
- int result = comparator.compare(keyIndex, mid);
- if (result < 0) {
- // the key is smaller
- high = mid - 1;
- } else if (result > 0) {
- // the key is larger
- low = mid + 1;
- } else {
- // an equal element is found,
- // continue to go right-ward
- ret = mid;
- low = mid + 1;
- }
- }
- return ret;
- }
-}
+/*
Review Comment:
This change is a bit odd, I cannot see a formatting here, though locally the spotless gives a warning about the file.
cc @danepitkin
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "laurentgo (via GitHub)" <gi...@apache.org>.
laurentgo commented on code in PR #40835:
URL: https://github.com/apache/arrow/pull/40835#discussion_r1567780737
##########
java/algorithm/pom.xml:
##########
@@ -20,6 +20,10 @@
<name>Arrow Algorithms</name>
<description>(Experimental/Contrib) A collection of algorithms for working with ValueVectors.</description>
+ <properties>
+ <spotless.version>2.42.0</spotless.version>
Review Comment:
Could we use spotless [2.30.0](https://www.atlassian.com/agile/project-management/epics-stories-themes) until the build uses a newer java version?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40819: [Java] Adding Spotless to Algorithm module [arrow]
Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on PR #40835:
URL: https://github.com/apache/arrow/pull/40835#issuecomment-2059685618
@laurentgo thanks for the feedback. I added a few quick comments for now, and I will address them accordingly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org