[MRG] Modification source_target_merge function behaviours

[YanisLalou]

Issue: #61
Change the name of source_target_merge
Allow an N dimension array as an input for source_target_merge
Add test cases

For source_target_merge:

• Sample domain input should be optional
• Y_target can be equal to None (check there's only one None given in all pairs)
• X_source/target can be list of arrays, check all sizes are coherent

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (52b63b0) 88.54% compared to head (4445f07) 88.83%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #71      +/-   ##
==========================================
+ Coverage   88.54%   88.83%   +0.29%     
==========================================
  Files          41       41              
  Lines        2601     2678      +77     
==========================================
+ Hits         2303     2379      +76     
- Misses        298      299       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kachayev]

I think we should match split API: accept as many pairs of 'things' as necessary, alongside with a named argument 'sample_domain'.

So I can do something like

X, y = source_target_merge(X_source, X_target, y_source, y_target, sample_domain=sample_domain)

[rflamary]

Do you mean :

X, y, sample_domain = source_target_merge(X_source, X_target, y_source, y_target, sample_domain)

insterad?

[YanisLalou]

In source_target_split() we don't output back the sample domain, just the splitted arrays. Makes sense to do the same here

[kachayev]

Right, as we have this function to pair up with the split functionality, we shouldn't encourage swapping sample_domain to a new one, even though it's technically possible (it would mean that we have to return it with every operation). Moreover, it might easily lead to 'accidental' loss of the information when having multiple sources/targets.

[YanisLalou]

For now source_target_merge() accepts an even number of arrays as input + the sample domain array.
It works by considering the arrays by duo (the first one is considered as the source and the second the target).
Thus I'm wondering:

• if it would be better to accept the duos as tuples rather than directly in the *arrays.
• Don't know if the doc is clear enough

Would love your feedbacks

[kachayev]

Regarding pairs, you are right, it might be better with pairs. But it's common to have pairs of ndarrays as an input type... So it might also be a bit confusing. Let's keep the current implementation to see how it works in practice. We will be able to change it if necessary.

[kachayev]

@YanisLalou is this one ready for review?

[YanisLalou]

@YanisLalou is this one ready for review?

Almost, there's one codevoc test that I still need to implement + check if I've managed to handle all possible cases.
But otherwise yes you can start reviewing it + if you find any edge cases that I didn't foreseen, that would really help me

[kachayev]

Overall comment, let's replace 'duo' with 'pair'. I mentioned it multiple times in different places in the code but certainly not everywhere.

[kachayev]

Great work, @YanisLalou!