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 logo and benchmarks to README #60

Merged
merged 7 commits into from
Apr 24, 2023
Merged

Conversation

eriknw
Copy link
Member

@eriknw eriknw commented Apr 15, 2023

This is a companion PR to python-graphblas/python-graphblas#432

I copied the logo from that PR and manually modified it.

@eriknw eriknw added the documentation Improve or add to documentation label Apr 15, 2023
@codecov-commenter
Copy link

codecov-commenter commented Apr 15, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (09776b6) 71.62% compared to head (e840706) 71.62%.

📣 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           @@
##             main      #60   +/-   ##
=======================================
  Coverage   71.62%   71.62%           
=======================================
  Files          89       89           
  Lines        3461     3461           
  Branches      642      642           
=======================================
  Hits         2479     2479           
  Misses        783      783           
  Partials      199      199           

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 in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@SultanOrazbayev SultanOrazbayev left a comment

Choose a reason for hiding this comment

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

This one also looks great, Erik! :)

Copy link
Member

Choose a reason for hiding this comment

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

It's a small thing, but the last letter in "graphblas" is a bit cropped out. I think it still works, though, so not a serious issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

Really? I don't think I see that. Can you take a screenshot of what you are seeing?

Copy link
Member

Choose a reason for hiding this comment

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

Screenshot 2023-04-18 at 09 11 26

I checked Chrome and Safari, both look like this for me...

Copy link
Member

Choose a reason for hiding this comment

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

And now that I'm looking at it again, the last s in algorithms is also gone... Interesting.

Copy link
Member

Choose a reason for hiding this comment

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

I tried playing around with zoom to see if that would make a diff, but nope.

Copy link
Member Author

Choose a reason for hiding this comment

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

I see this now (on my macbook); it renders fine for me on Windows where I created it. Thanks for letting me know!

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, I think the logo should be fixed now if you'd like to take a look to verify.

Copy link
Member

Choose a reason for hiding this comment

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

To recap testing on my end: looks good on MacOS and Windows, looks truncated on Ubuntu.

@SultanOrazbayev
Copy link
Member

SultanOrazbayev commented Apr 21, 2023 via email

@eriknw
Copy link
Member Author

eriknw commented Apr 21, 2023

nope looks the same

Dope! Thanks for checking. Jim and I have learned that the font looks very different on different systems (Windows, Mac, Android, etc). So, I think @jim22k is going to convert all the svg logos to PNGs (Jim, don't forget to minimize the PNGs--such as from https://compresspng.com/ and https://tinypng.com/ --to improve page load times!).

@jim22k
Copy link
Member

jim22k commented Apr 22, 2023

I switched from svg to png. This bumps the logo size from 1k to 35k, but I think it's worth it to avoid the need to grab fonts from the local machine, which leads to different looks and widths. PNG will always be consistent.

Copy link
Member

@SultanOrazbayev SultanOrazbayev left a comment

Choose a reason for hiding this comment

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

Looks good! Interesting the file size is so large (35K), but small enough to not be an issue.

@eriknw
Copy link
Member Author

eriknw commented Apr 22, 2023

Thanks for converting the svg font to paths @jim22k. I minified the svg (that you converted the font into paths) with https://vecta.io/nano and deleted the png, b/c the svg looks better.

I think this is good to go in 🎉

Now let's do the same for the svgs in python-graphblas :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improve or add to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants