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

FIX Edit style of model plot to fit in the page #178

Merged
merged 3 commits into from
Oct 25, 2022

Conversation

0xrushi
Copy link
Contributor

@0xrushi 0xrushi commented Oct 8, 2022

How it will look:

Readme example here :- https://huggingface.co/rushic24/TestPlaygroundSkops

Fixes #164

Copy link
Member

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @rushic24. We don't have a testing framework for these issues, but could you please create a model repo with this change so that we can see the change in action?

skops/card/_model_card.py Outdated Show resolved Hide resolved
@adrinjalali
Copy link
Member

You also need to apply linting (black and isort, although only black is relevant in this case).

You can enable them in your pre-commit hooks as explain here: https://github.com/skops-dev/skops/blob/main/CONTRIBUTING.rst#using-condamamba

@merveenoyan
Copy link
Collaborator

merveenoyan commented Oct 10, 2022

Thanks a lot for the fix, looks neat!
(x-posting from the issue) @rushic24 given this is about model card README in 🤗 Hub it would be nice to have a model with a lot of preprocessors in the pipeline (given it's making the plot bigger) inside a model repository that we can look at.

Copy link
Collaborator

@merveenoyan merveenoyan left a comment

Choose a reason for hiding this comment

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

Thanks a lot for working on it 🤗

skops/card/_model_card.py Outdated Show resolved Hide resolved
@merveenoyan
Copy link
Collaborator

@rushic24 Lastly, can you post two repositories, one that has a big pipeline plot and another one that has a small one? (given we can't check how Hub looks on our side 😅 just to make sure both cases are fine!)

@0xrushi
Copy link
Contributor Author

0xrushi commented Oct 24, 2022

Added in the Readme below https://huggingface.co/rushic24/TestPlaygroundSkops
Are the macos tests failing a serious issue?

@adrinjalali
Copy link
Member

It looks a bit narrow to me, as in, the container seems to have more space on the right side that we could use, but I'm not sure how easy it is to fix that and if it's not, I'm happy with the solution here.

@merveenoyan
Copy link
Collaborator

@adrinjalali it ends where the right most boundary of other components end, no? Added a photo below: image
I think it's good.

@adrinjalali adrinjalali changed the title Fix: #164 Edit style of model plot FIX Edit style of model plot to fit in the page Oct 25, 2022
@adrinjalali adrinjalali merged commit bd53e8c into skops-dev:main Oct 25, 2022
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.

Edit style of model plot to fit inside the model card
3 participants