FEAT-#3451: Support __partitioned__ protocol

[YarShev]

Signed-off-by: Igoshev, Yaroslav [email protected]

What do these changes do?

• commit message follows format outlined here
• passes flake8 modin
• passes black --check modin
• signed commit with git commit -s
• Resolves Support __partitioned__ protocol #3451
• tests passing
• module layout described at docs/developer/architecture.rst is up-to-date

[codecov]

Codecov Report

Merging #3452 (e460612) into master (7727c23) will decrease coverage by 34.44%.
The diff coverage is 27.57%.

❗ Current head e460612 differs from pull request most recent head 1caf113. Consider uploading reports for the commit 1caf113 to get more accurate results

@@             Coverage Diff             @@
##           master    #3452       +/-   ##
===========================================
- Coverage   83.23%   48.79%   -34.45%     
===========================================
  Files         147      144        -3     
  Lines       15246    15529      +283     
===========================================
- Hits        12690     7577     -5113     
- Misses       2556     7952     +5396     

Impacted Files	Coverage Δ
modin/backends/base/query_compiler.py	62.47% <ø> (-36.81%)	⬇️
modin/conftest.py	55.70% <ø> (-25.88%)	⬇️
modin/distributed/dataframe/pandas/partitions.py	0.00% <0.00%> (-24.40%)	⬇️
...ngines/base/io/column_stores/parquet_dispatcher.py	21.21% <0.00%> (-68.62%)	⬇️
modin/engines/base/io/text/csv_glob_dispatcher.py	10.62% <0.00%> (-67.88%)	⬇️
modin/engines/base/io/text/excel_dispatcher.py	9.16% <0.00%> (-82.18%)	⬇️
modin/engines/base/io/text/fwf_dispatcher.py	9.09% <0.00%> (-78.09%)	⬇️
modin/engines/base/io/text/json_dispatcher.py	20.45% <0.00%> (-75.70%)	⬇️
...in/experimental/backends/omnisci/query_compiler.py	0.00% <0.00%> (-89.33%)	⬇️
modin/experimental/cloud/omnisci.py	0.00% <0.00%> (ø)
... and 131 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7727c23...1caf113. Read the comment docs.

[devin-petersohn]

Is this a high priority? What consumers of this type of protocol already exist?

[Garra1980]

yeah, let's not consider this for 0.11

[fschlimb]

@devin-petersohn You're raising a chicken-and-egg issue.

Generally, adding support as a consumer is more involved but at the same time adds more value. Adding this at the producer side is less involved. Like here it is very isolated - it has no effect on anything else.

Current Implementation status is

• Supporting __partitioned__ interface ray-project/xgboost_ray#153
• Interface for DPPY interoperability helmholtz-analytics/heat#788
• https://github.com/fschlimb/daal4py/tree/feature/partitioned_interface
• FEAT-#3451: Support __partitioned__ protocol #3452

More to come, but we need producers! I'd appreciate seeing modin as a producer.

To add more value for modin, we can add support for consuming this API. A simple "from_partitioned" would provide the basic functionality. "Automatic" detection when accepting data at construction time would be nice to have and could be added later.

[YarShev]

To add more value for modin, we can add support for consuming this API. A simple "from_partitioned" would provide the basic functionality. "Automatic" detection when accepting data at construction time would be nice to have and could be added later.

It would also help to match partitions of the left object (Modin DF/Series) and partitions of the right object (an instance supporting __partitioned__) when performing operations that may require right object(s).

[devin-petersohn]

Generally, adding support as a consumer is more involved but at the same time adds more value. Adding this at the producer side is less involved. Like here it is very isolated - it has no effect on anything else.

It's not a big deal to add something, but I don't want code bloat for things that won't be useful. I think it's a good idea to have a protocol like this, but who would use it? What library has said that they actually need an interface like this to be able to work with Modin? We at least need to know if it would be useful to a meaningful consumer before we add it.

Typically these protocols have significant amounts of input from multiple interested parties (producers and consumers) to answer these questions before they are even designed. I just want to make sure we aren't adding something that won't be used. I don't deny the usefulness of this protocol, but whether or not it will be used is still not answered to me.

[fschlimb]

Typically these protocols have significant amounts of input from multiple interested parties (producers and consumers) to answer these questions before they are even designed.

Yes, we are open to feedback and suggestions for different designs and features. As mentioned above, your feedback/suggestions is/are highly appreciated. Talking about something concrete is usually easier than keeping discussions in the abstract.

I just want to make sure we aren't adding something that won't be used. I don't deny the usefulness of this protocol, but whether or not it will be used is still not answered to me.

That's of course a valid request.

One issue in the process of ramping this up is that we are targeting something that is currently less of a concern but we want to make sure we can avoid running into a situation where implementations of distributed features become messy.

It's not a big deal to add something, but I don't want code bloat for things that won't be useful. I think it's a good idea to have a protocol like this, but who would use it? What library has said that they actually need an interface like this to be able to work with Modin? We at least need to know if it would be useful to a meaningful consumer before we add it.

The idea is to allow a packages to consume various distributed containers. It is not meant to be an enabler for modin specifically. Of course packages like xgboost_ray can support modin/RayDataSetl/MLDataSet/HeAT/... by implementing dedicated code for each. Avoiding such specialization for each (in particular upcoming) structure is the major motivation for this.

[YarShev]

A new discussion was initiated with the data-API consortium: data-apis/consortium-feedback#7

[vnlitvinov]

@YarShev should we mark this as draft then?

[YarShev]

@vnlitvinov , yes, we can mark the PR as a draft for now. I'll update it once the protocol settles.

[YarShev]

Closing this PR as no plans on pushing this forward.