kou commented on code in PR #41494:
URL: https://github.com/apache/arrow/pull/41494#discussion_r1600289742


##########
.pre-commit-config.yaml:
##########
@@ -123,6 +123,7 @@ repos:
           ?^go/.*/CMakeLists\.txt$|
           ?^java/.*/CMakeLists\.txt$|
           ?^matlab/.*/CMakeLists\.txt$|
+          ?^python/CMakeLists\.txt$|

Review Comment:
   Oh... These patterns are wrong...
   
   Could you use `^XXX/.*CMakeLists\.txt$|` for all existing patterns too?



##########
python/setup.py:
##########
@@ -152,48 +152,27 @@ def initialize_options(self):
             if not hasattr(sys, 'gettotalrefcount'):
                 self.build_type = 'release'
 
-        self.with_azure = strtobool(
-            os.environ.get('PYARROW_WITH_AZURE', '0'))
-        self.with_gcs = strtobool(
-            os.environ.get('PYARROW_WITH_GCS', '0'))
-        self.with_s3 = strtobool(
-            os.environ.get('PYARROW_WITH_S3', '0'))
-        self.with_hdfs = strtobool(
-            os.environ.get('PYARROW_WITH_HDFS', '0'))
-        self.with_cuda = strtobool(
-            os.environ.get('PYARROW_WITH_CUDA', '0'))
-        self.with_substrait = strtobool(
-            os.environ.get('PYARROW_WITH_SUBSTRAIT', '0'))
-        self.with_flight = strtobool(
-            os.environ.get('PYARROW_WITH_FLIGHT', '0'))
-        self.with_acero = strtobool(
-            os.environ.get('PYARROW_WITH_ACERO', '0'))
-        self.with_dataset = strtobool(
-            os.environ.get('PYARROW_WITH_DATASET', '0'))
-        self.with_parquet = strtobool(
-            os.environ.get('PYARROW_WITH_PARQUET', '0'))
-        self.with_parquet_encryption = strtobool(
-            os.environ.get('PYARROW_WITH_PARQUET_ENCRYPTION', '0'))
-        self.with_orc = strtobool(
-            os.environ.get('PYARROW_WITH_ORC', '0'))
-        self.with_gandiva = strtobool(
-            os.environ.get('PYARROW_WITH_GANDIVA', '0'))
+        self.with_azure = None

Review Comment:
   Do we still need `self.with_XXX`? It seems that nobody sets them because we 
remove `os.environ.get(...)`.



##########
python/setup.py:
##########
@@ -329,54 +315,8 @@ def append_cmake_bool(value, varname):
             self._found_names = []
             for name in self.CYTHON_MODULE_NAMES:
                 built_path = pjoin(install_prefix, name + ext_suffix)
-                if not os.path.exists(built_path):
-                    print(f'Did not find {built_path}')
-                    if self._failure_permitted(name):
-                        print(f'Cython module {name} failure permitted')
-                        continue
-                    raise RuntimeError('PyArrow C-extension failed to build:',
-                                       os.path.abspath(built_path))
-
-                self._found_names.append(name)
-
-    def _failure_permitted(self, name):
-        if name == '_parquet' and not self.with_parquet:
-            return True
-        if name == '_parquet_encryption' and not self.with_parquet_encryption:
-            return True
-        if name == '_orc' and not self.with_orc:
-            return True
-        if name == '_flight' and not self.with_flight:
-            return True
-        if name == '_substrait' and not self.with_substrait:
-            return True
-        if name == '_azurefs' and not self.with_azure:
-            return True
-        if name == '_gcsfs' and not self.with_gcs:
-            return True
-        if name == '_s3fs' and not self.with_s3:
-            return True
-        if name == '_hdfs' and not self.with_hdfs:
-            return True
-        if name == '_dataset' and not self.with_dataset:
-            return True
-        if name == '_acero' and not self.with_acero:
-            return True
-        if name == '_exec_plan' and not self.with_acero:
-            return True
-        if name == '_dataset_orc' and not (
-                self.with_orc and self.with_dataset
-        ):
-            return True
-        if name == '_dataset_parquet' and not (
-                self.with_parquet and self.with_dataset
-        ):
-            return True
-        if name == '_cuda' and not self.with_cuda:
-            return True
-        if name == 'gandiva' and not self.with_gandiva:
-            return True
-        return False
+                if os.path.exists(built_path):
+                    self._found_names.append(name)

Review Comment:
   Do we need to prepare `self._found_names`?
   It seems that nobody uses it.



-- 
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-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to