bkietz commented on a change in pull request #7156:
URL: https://github.com/apache/arrow/pull/7156#discussion_r434077838



##########
File path: cpp/src/arrow/dataset/discovery.cc
##########
@@ -102,11 +102,10 @@ Result<std::shared_ptr<Dataset>> 
UnionDatasetFactory::Finish(FinishOptions optio
   return std::shared_ptr<Dataset>(new UnionDataset(options.schema, 
std::move(children)));
 }
 
-FileSystemDatasetFactory::FileSystemDatasetFactory(
-    std::vector<std::string> paths, std::shared_ptr<fs::FileSystem> filesystem,
-    std::shared_ptr<FileFormat> format, FileSystemFactoryOptions options)
-    : paths_(std::move(paths)),
-      fs_(std::move(filesystem)),
+FileSystemDatasetFactory::FileSystemDatasetFactory(std::vector<FileSource> 
sources,
+                                                   std::shared_ptr<FileFormat> 
format,
+                                                   FileSystemFactoryOptions 
options)

Review comment:
       After this PR, what would be the advantage of that? 
`FileSystemDatasetFactory.__init__` is the only function which calls 
`CFileSystemDatasetFactory.MakeFromSources` so I think it would also be the 
only function which would use `CFileSystemDatasetFactory.MakeFromPaths`. I 
don't think it'll save any boilerplate




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to