Style concats
State: closed

- [x] closes #49207
- [x] [Tests added and passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#writing-tests) if fixing a bug or adding a new feature
- [x] All [code checks passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#pre-commit).
- [x] Added [type annotations](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#type-hints) to new arguments/methods/functions.
- [x] Added an entry in the `doc/source/whatsnew/v1.5.2.rst` file if fixing a bug or adding a new feature.


Labels:
name: Styler
description: conditional formatting using DataFrame.style

Comments:
On question before reviewing this properly.

How does this impact the CSS class names of the concatenated stylers?
From memory, you have "data" and "foot_data" if you use
`styler.concat(other_styler)`
If you do:
`styler.concat(other_styler.concat(third_styler))`
you have  "data" and "foot_data" and "foot_foot_data".

Using your PR i think there is a difference between 

`styler.concat(other_styler.concat(third_styler))` and `styler.concat(other_styler).concat(third_styler)` in terms of these.
These must be documented as well as the underlying functionalioty

It's true. In the original implementation, you'll have _foot got the first concat, _foot_foot for the second, etc.
In this implementation, you'll have just one `_foot` for all the styles concatenated (at the top level) via chaining.

Should we make them all different? I can try

Also, I haven't seen the original behavior documented anywhere. Have I missed something, or should we document the original behavior also? If so, where?
No I dont think we should make them different. I actually like the way you have implemented this so that you can chain, and any concatenated `other` will have the same "foot_" prefix.

The ability to create "foot_foot_" is still available due the flexible way this has been implemented.

No this was not documented previously as your issue has raised. With the "notes" section of the `styler.concat` docs there should be a section on concatenating multiple stylers where these issues are explained.
Sorry for the delay, I'm definitely want to push this, but didn't had time.
Will try to look at it today/tomorrow

On Wed, 2 Nov 2022, 17:01 JHM Darbyshire, ***@***.***> wrote:

> ***@***.**** commented on this pull request.
> ------------------------------
>
> In pandas/io/formats/style_render.py
> <https://github.com/pandas-dev/pandas/pull/49212#discussion_r1011896919>:
>
> >
> -            for (r, c), v in self.concatenated.ctx.items():
> +            for (r, c), v in concatenated.ctx.items():
>
> @tsvikas <https://github.com/tsvikas> what do you think of above? This is
> a good PR so I want to follow up.
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/49212#discussion_r1011896919>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AESBIZOATDTZXI6DQSFTSVDWGJ66HANCNFSM6AAAAAARKKLHUM>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>

Sorry for the delay. 
I will separate my response into several sections. Hopefully, it'll be more clear that way.

### 1. I've improved the tests
Now all 3 tests are concatenating 3 **different** styles, and verifying that they render as expected. They all pass.

### 2. GHA reports that "Some checks were not successful"
> Ubuntu / actions-310.yaml pyarrow=8 not single_cpu (pull_request) Failing after 91m

I have no idea why. I wouldn't expect the tests to pass on all the other systems and fail on this. 
Is this a known issue? Can anyone help me investigate?

### 3. updating `concatenated.ctx` inside the loop

> i dont think this will work correctly. The idea here is that the context object on `self` (the main styler) is updated for the additional rows provided by the other styler, hence why the row number is added to `len(self.styler)`. For adding a list of concatenated styler you will probably need to keep a counter of the rows for all of them.

I will answer in more detail inside the code conversation, but basically - It seems that you are correct. I've tried to code it and it failed the tests. I'm not sure why.

### 4. I've updated the docs.
hopefully it's OK.

### 5. same `foot_` prefix is used by all concatenated styles

> No I dont think we should make them different. I actually like the way you have implemented this so that you can chain, and any concatenated `other` will have the same "foot_" prefix.
> 
> The ability to create "foot_foot_" is still available due the flexible way this has been implemented.
> 
> No this was not documented previously as your issue has raised. With the "notes" section of the `styler.concat` docs there should be a section on concatenating multiple stylers where these issues are explained.

I'm not sure what is this prefix used for, but maybe differentiating between concatenated styles is important? 
maybe we can use `foot0_`, `foot1_`, etc. for the concatenated styles in the chain? I assume it'll be easy to implement.
(and `s1.concat(s2.concat(s3))` will have a `foot0_foot0_` prefix).

In any case, the docs are updated with the current behavior.
### 3. updating concatenated.ctx inside the loop
is fixed now.
I fixed everything except the __copy__ issue, and did a rebase (hopefully it's ok).
@attack68 should i move the whatsnew from 1.5.2 to 1.5.3? or wait after the review? or is someone else in care of that?
> @attack68 should i move the whatsnew from 1.5.2 to 1.5.3? or wait after the review? or is someone else in care of that?

you will need to change it now to 1.5.3, I believe 1.5.2 is in the release process so this wont make it.
mypy is now failing linting on some of the artifacts here.. can you address? 
@attack68 mypy fixed. there is another check (doc build and upload) that failed (some warning about numpy deprecation maybe?), and i don't know how to fix that

Sorry @tsvikas, the whats new is now out of date and needs a merge, although my changes above fixed the checks.

I have been waiting to see if anyone else wants to review, but the code here is quite restricted just to Styler.concat and doesnt affect anything else and is a good extension and fix so I will merge after it s updated.
i fixed the merge conlict in the `whatsnew`
i'm not sure why `ci/circleci: test-arm` is red

the failure is unrelated: `test_arrow_array`
Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

1. Checkout backport branch and update it.

```
git checkout 1.5.x
git pull
```

2. Cherry pick the first parent branch of the this PR on top of the older branch:
```
git cherry-pick -x -m1 7ef6a71c5c41eadf9ebea514f4be502e83c95a6f
```

3. You will likely have some merge/cherry-pick conflict here, fix them and commit:

```
git commit -am 'Backport PR #49212: Style concats'
```

4. Push to a named branch:

```
git push YOURFORK 1.5.x:auto-backport-of-pr-49212-on-1.5.x
```

5. Create a PR against branch 1.5.x, I would have named this PR:

> "Backport PR #49212 on branch 1.5.x (Style concats)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the `Still Needs Manual Backport` label once the PR gets merged.

If these instructions are inaccurate, feel free to [suggest an improvement](https://github.com/MeeseeksBox/MeeseeksDev).
                
@tsvikas would you have time to follow the backport instructions above to push the code to the 1.5.x branch?
> And apply the correct labels and milestones.
> 
> Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!
> 
> Remember to remove the `Still Needs Manual Backport` label once the PR gets merged.

done, up to "And apply the correct labels and milestones." and "remove the Still Needs Manual Backport label" (which i don't have permission to).


done, thx