Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for styler pandas object #66

Merged
merged 13 commits into from
Feb 19, 2023
Merged

Add support for styler pandas object #66

merged 13 commits into from
Feb 19, 2023

Conversation

ejbills
Copy link
Contributor

@ejbills ejbills commented Dec 20, 2022

See here :Styler object docs

This PR uses css-inline as a way to convert CSS styling into inline CSS styling for wider support on mail clients (<style> tags are not supported on clients like gmail and whatnot). This allows further customization for pandas dataframe tables in converting them to HTML with stylized attributes as seen in the doc link above.

This code does not affect how dataframes are handled currently, and only takes care of adding support for the Styler object.

@@ -63,6 +66,12 @@ def render_table(self, tbl, extra=None):
raise ImportError("Missing package 'pandas'. Prettifying tables requires Pandas.")

extra = {} if extra is None else extra

# Allow for pandas styler object, convert to inline CSS for email client rendering
Copy link
Contributor Author

@ejbills ejbills Dec 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this also takes care of the "TODO: Nicer tables." in the function, but I will leave that to your discretion to remove.

@ejbills
Copy link
Contributor Author

ejbills commented Dec 22, 2022

Check out this example of how far the stylizing of tables can go with this simple addition. Also, ignore the data in the table as it is just an ongoing project I am extensively utilizing redmail for.

image

This is achieved with the following code:

styles = [
        dict(selector="tr:hover",
                    props=[("background", "#f4f4f4")]),
        dict(selector="th", props=[("color", "#fff"),
                                   ("border", "1px solid #eee"),
                                   ("padding", "12px 35px"),
                                   ("border-collapse", "collapse"),
                                   ("background", "#00cccc"),
                                   ("text-transform", "uppercase"),
                                   ("font-size", "18px")
                                   ]),
        dict(selector="td", props=[("color", "#999"),
                                   ("border", "1px solid #eee"),
                                   ("padding", "12px 35px"),
                                   ("border-collapse", "collapse"),
                                   ("font-size", "15px")
                                   ]),
        dict(selector="table", props=[
                                        ("font-family" , 'Arial'),
                                        ("margin" , "25px auto"),
                                        ("border-collapse" , "collapse"),
                                        ("border" , "1px solid #eee"),
                                        ("border-bottom" , "2px solid #00cccc"),
                                          ]),
        dict(selector="caption", props=[("caption-side", "top")])
    ]

    style = df.style.set_table_styles(styles).hide(axis="index").set_caption("This is an example of a formatted table using pandas Stylers with redmail!")

Then you just pass it the same as a normal pandas dataframe:
body_tables = { table = style },

It's that easy and brings a whole world of customization to the tables within emails seamlessly with the preexisting redmail table handler. Let me know if you have any questions @Miksus!

@ejbills
Copy link
Contributor Author

ejbills commented Feb 14, 2023

@Miksus bumping

@Waghabond
Copy link
Contributor

wow nice PR!

@ejbills
Copy link
Contributor Author

ejbills commented Feb 14, 2023

wow nice PR!

Thanks! I am unable to request your review to have it merged. Either way, feel free to implement this another way if you see fit, takes a couple lines and makes customization really extensive.

@Miksus
Copy link
Owner

Miksus commented Feb 18, 2023

Sorry for taking an eternity to review this (and bunch of other issues and PRs). Have had a bit longer Christmas break from open-source just because I have been busy with hobbies and random events. I wanted to have a full free day to focus on my open-source projects to get back on track with all the amazing PRs and issues I have ignored but seems I have to work on the time I have.

This indeed seems promising. The old styling was quite ugly and unreliable. The only issue I see is the Pandas dependency. Some people want Pandas to be an optional dependency (as it is quite heavy).

One candidate would be similar setup as I did with Red Bird (it has this small utilility). Perhaps a similar setup would make the optional imports smooth here as well? Another option would be a simply try-except.

We also need the css_inline here: https://github.com/Miksus/red-mail/blob/master/pyproject.toml#L36.

I have a random event today (again...) but I'll take this as a priority now.

@Miksus
Copy link
Owner

Miksus commented Feb 18, 2023

Perhaps the css_inline could be an optional dependency as well. With a quick glance, the project is somewhat in use but the code coverage is 50% so perhaps it's best not to force users to have it.

I hope you don't mind if I tinker with your PR.

@Miksus
Copy link
Owner

Miksus commented Feb 18, 2023

Having the Pandas Styler as an optional dependency is a bit tricky but I'm sure we will come up with an idea.

@Miksus
Copy link
Owner

Miksus commented Feb 18, 2023

Oh, I'm a bit stupid. This section of the code couldn't even be executed if Pandas was missing. I think we can put the import directly to the function. It's ugly but in this sort of situation, there is no better alternative.

@Waghabond
Copy link
Contributor

Perhaps the css_inline could be an optional dependency as well.

Bit unfortunate that this package my have to resort to using the new css-inline package, i'm sure it will be really good once it starts getting used more widely. However, the older premailer package was battle tested because it had been around for a long time and it was a lot more feature rich and more powerful (though it was a lot slower as a result of being written in python instead of rust). Cant really use it now though when it comes to developing new packages because the latest version of python it officially supports in 3.8 and it hasn't been updated since 2021

@Miksus
Copy link
Owner

Miksus commented Feb 18, 2023

Ye, it seems it's still active (the last merge was 3 weeks ago). Anyways as this is a feature that does not change any previous behaviour, I don't see much risk in terms of the dependency.

However, the existing Pandas table "styling" is pretty ugly so it would be nice to rewrite that with such a library as well. I initially copied the pattern from Pandas itself which was pure dog shit, to be honest.

@codecov-commenter
Copy link

codecov-commenter commented Feb 18, 2023

Codecov Report

Base: 99.68% // Head: 99.68% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (f1ac8fc) compared to base (a813ac1).
Patch coverage: 100.00% of modified lines in pull request are covered.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #66   +/-   ##
=======================================
  Coverage   99.68%   99.68%           
=======================================
  Files          13       13           
  Lines         627      635    +8     
=======================================
+ Hits          625      633    +8     
  Misses          2        2           
Impacted Files Coverage Δ
redmail/email/body.py 100.00% <100.00%> (ø)
redmail/email/utils.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ejbills
Copy link
Contributor Author

ejbills commented Feb 18, 2023

Perhaps the css_inline could be an optional dependency as well.

Bit unfortunate that this package my have to resort to using the new css-inline package, i'm sure it will be really good once it starts getting used more widely. However, the older premailer package was battle tested because it had been around for a long time and it was a lot more feature rich and more powerful (though it was a lot slower as a result of being written in python instead of rust). Cant really use it now though when it comes to developing new packages because the latest version of python it officially supports in 3.8 and it hasn't been updated since 2021

Only reason I went with css-inline and not premailer is because the css-inline package converts nth-child elements to be renderable in email clients that do not support it, which is a majority of clients (docs). I really like the alternating gray color from the built in current redmail table and wanted that to still be an option. Obviously if it is only updated till 3.8 then I agree we should switch it to premailer.

I hope you don't mind if I tinker with your PR.

Feel free to do whatever you want to the PR. Since it's only a couple lines it doesn't affect much and I see I missed some definitions for dependencies, sorry about that!

@Miksus
Copy link
Owner

Miksus commented Feb 18, 2023

Feel free to do whatever you want to the PR. Since it's only a couple lines it doesn't affect much and I see I missed some definitions for dependencies, sorry about that!

Nah, no problem! I find it fun playing with this and I really appreciate you decided to make the PR in the first place! I'll tinker with this a bit more, write some documentation and fix that one test, and we are good to go.

Are you working on this as well at the moment? Just wondering I don't mess with your changes.

@ejbills
Copy link
Contributor Author

ejbills commented Feb 18, 2023

Are you working on this as well at the moment? Just wondering I don't mess with your changes.

Nope! You are all good, I will let you work out the kinks. Glad I was able to contribute a little bit!

@Waghabond
Copy link
Contributor

Obviously if it is only updated till 3.8 then I agree we should switch it to premailer.

Actually premailer is the one that hasn't been updated since Python 3.8. css inline seems good except for it's low test coverage statistics.

@Miksus
Copy link
Owner

Miksus commented Feb 18, 2023

Yep, I think this is good to go! I'll sleep the night though and double-check in the morning. The master is auto-deployed to test PyPI so if you need this quickly, it should be installable from there. I possibly will do a formal release tomorrow though.

The css_inline seems quite promising. I was quite surprised at how well it seems to work.

Thanks again, @ejbills, for the effort and patience and sorry for the delay. Thank you, @Waghabond, also for being active and for helping to push this through!

@Miksus Miksus merged commit 0a6f0a6 into Miksus:master Feb 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants