GH-39440: [Python] Calling pyarrow.dataset.ParquetFileFormat.make_write_options as a class method results in a segfault

[AlenkaF]

Rationale for this change

Calling make_write_options() method as class instead of instance method results in segfault.

What changes are included in this PR?

Adds a type check on self and raises an error if not ParquetFileFormat.

Are these changes tested?

Yes.

Are there any user-facing changes?

No.

• GitHub Issue: [Python] Calling pyarrow.dataset.ParquetFileFormat.make_write_options as a class method results in a segfault #39440

[danepitkin]

LGTM!

[pitrou]

I don't think it makes sense to add the check here, because we would have to do that on many existing methods. Example:

>>> pa.Array.get_total_buffer_size(0)
Erreur de segmentation

[AlenkaF]

Right, but I would also agree with Joris here: #39440 (comment)

Now, in this case of ParquetFileFormat.make_write_options, it's more important I think, because it makes more sense to indeed try to call this thinking it is a static class method.

[pitrou]

I opened cython/cython#6127 on the Cython side. Remember that it's fine to suggest additions to third-party libraries or utilities when it's general enough that it could serve other people.

[pitrou]

Right, but I would also agree with Joris here: #39440 (comment)

Now, in this case of ParquetFileFormat.make_write_options, it's more important I think, because it makes more sense to indeed try to call this thinking it is a static class method.

Ah! It makes sense, then. Can you add the rationale as a comment in the source code?

[AlenkaF]

I opened cython/cython#6127 on the Cython side. Remember that it's fine to suggest additions to third-party libraries or utilities when it's general enough that it could serve other people.

Oh cool! Broadening my horizons =)

[AlenkaF]

@pitrou I have added a comment in the last commit: b09cae7.

[conbench-apache-arrow]

After merging your PR, Conbench analyzed the 7 benchmarking runs that have been run so far on merge-commit 074d45f.

There were 4 benchmark results indicating a performance regression:

• Commit Run on ursa-i9-9960x at 2024-04-05 09:14:51Z
  • dataset-selectivity (Python) with dataset=nyctaxi_multi_ipc_s3, selectivity=10%
  • dataset-selectivity (Python) with dataset=nyctaxi_multi_parquet_s3, selectivity=10%
• and 2 more (see the report linked below)

The full Conbench report has more details. It also includes information about 4 possible false positives for unstable benchmarks that are known to sometimes produce them.