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

[WIP] make repr formatting narrower #903

Merged
merged 7 commits into from
Feb 28, 2023
Merged

Conversation

davanstrien
Copy link
Member

What this PR does?

This PR aims to make the __repr__s in hf_api.py easier to view on a narrow screen i.e. to favour a longer __repr__ for a class over a short but very wide __repr__. This PR closes #857

Intended outcome

As discussed in #857, the goal here is to have a __repr__ that is easy to view on a narrow screen without horizontal scrolling.

I have tried to make sure the approach taken doesn't do anything esoteric and doesn't add too much additional code to the __repr__. I have tried to keep the format of the __repr__ as similar to what is there now, except for making it narrower.

Approach taken

For now, I have just changed the __repr__ for ModelInfo to get initial feedback. The __repr__ now uses pprint.pformat to offload the formatting work for the attributes inside the class. At the moment, the __repr__s for all the classes in hf_api.py show all the attributes, but it would also be possible to hide some before passing to pformat if needed. I chose this approach since many people working with python will be at least vaguely familiar with pprint so hopefully, the code is quite easily understood.

My suggestion, if this approach/format looks okay, would be to add this to all the classes in hf_api. There is also the option of tweaking the width parameter used by pformat.

An example of the output given by the old and new __repr__ are shown below.

old

ModelInfo: {
	modelId: flyswot/convnext-tiny-224_flyswot
	sha: c6d4b2138e10efeafef8f5305ce16270ca583618
	lastModified: 2022-04-05T16:08:35.000Z
	tags: ['pytorch', 'convnext', 'image-classification', 'dataset:image_folder', 'transformers', 'generated_from_trainer', 'model-index']
	pipeline_tag: image-classification
	siblings: [RepoFile(rfilename='.gitattributes'), RepoFile(rfilename='.gitignore'), RepoFile(rfilename='README.md'), RepoFile(rfilename='config.json'), RepoFile(rfilename='preprocessor_config.json'), RepoFile(rfilename='pytorch_model.bin'), RepoFile(rfilename='training_args.bin')]
	private: False
	author: flyswot
	config: {'architectures': ['ConvNextForImageClassification'], 'model_type': 'convnext'a
	id: flyswot/convnext-tiny-224_flyswot
	downloads: 984
	library_name: transformers
	model-index: [{'name': 'convnext-tiny-224_flyswot', 'results': [{'task': {'name': 'Image Classification', 'type': 'image-classification'}, 'dataset': {'name': 'image_folder', 'type': 'image_folder', 'args': 'default'}, 'metrics': [{'name': 'F1', 'type': 'f1', 'value': 0.9756290792360154}]}]}]
	cardData: {'tags': ['generated_from_trainer'], 'datasets': ['image_folder'], 'metrics': ['f1'], 'model-index': [{'name': 'convnext-tiny-224_flyswot', 'results': [{'task': {'name': 'Image Classification', 'type': 'image-classification'}, 'dataset': {'name': 'image_folder', 'type': 'image_folder', 'args': 'default'}, 'metrics': [{'name': 'F1', 'type': 'f1', 'value': 0.9756290792360154}]}]}]}
	transformersInfo: {'auto_model': 'AutoModelForImageClassification', 'pipeline_tag': 'image-classification', 'processor': 'AutoFeatureExtractor'}
}

new

ModelInfo: { 
        {'modelId': 'flyswot/convnext-tiny-224_flyswot',
         'sha': 'c6d4b2138e10efeafef8f5305ce16270ca583618',
         'lastModified': '2022-04-05T16:08:35.000Z',
         'tags': ['pytorch',
                  'convnext',
                  'image-classification',
                  'dataset:image_folder',
                  'transformers',
                  'generated_from_trainer',
                  'model-index'],
         'pipeline_tag': 'image-classification',
         'siblings': [RepoFile(rfilename='.gitattributes'),
                      RepoFile(rfilename='.gitignore'),
                      RepoFile(rfilename='README.md'),
                      RepoFile(rfilename='config.json'),
                      RepoFile(rfilename='preprocessor_config.json'),
                      RepoFile(rfilename='pytorch_model.bin'),
                      RepoFile(rfilename='training_args.bin')],
         'private': False,
         'author': 'flyswot',
         'config': {'architectures': ['ConvNextForImageClassification'],
                    'model_type': 'convnext'},
         'id': 'flyswot/convnext-tiny-224_flyswot',
         'downloads': 984,
         'library_name': 'transformers',
         'model-index': [{'name': 'convnext-tiny-224_flyswot',
                          'results': [{'task': {'name': 'Image Classification',
                                                'type': 'image-classification'},
                                       'dataset': {'name': 'image_folder',
                                                   'type': 'image_folder',
                                                   'args': 'default'},
                                       'metrics': [{'name': 'F1',
                                                    'type': 'f1',
                                                    'value': 0.9756290792360154}]}]}],
         'cardData': {'tags': ['generated_from_trainer'],
                      'datasets': ['image_folder'],
                      'metrics': ['f1'],
                      'model-index': [{'name': 'convnext-tiny-224_flyswot',
                                       'results': [{'task': {'name': 'Image Classification',
                                                             'type': 'image-classification'},
                                                    'dataset': {'name': 'image_folder',
                                                                'type': 'image_folder',
                                                                'args': 'default'},
                                                    'metrics': [{'name': 'F1',
                                                                 'type': 'f1',
                                                                 'value': 0.9756290792360154}]}]}]},
         'transformersInfo': {'auto_model': 'AutoModelForImageClassification',
                              'pipeline_tag': 'image-classification',
                              'processor': 'AutoFeatureExtractor'}}
	}

@davanstrien
Copy link
Member Author

@Wauplin
Copy link
Contributor

Wauplin commented Aug 12, 2022

Hi @davanstrien thanks for your PR. I want to follow-up on it. Is it still a Draft/WIP work or it's ready to review/merge ?
(cc @julien-c @LysandreJik as well)

@davanstrien
Copy link
Member Author

davanstrien commented Aug 12, 2022

Hi @davanstrien thanks for your PR. I want to follow-up on it. Is it still a Draft/WIP work or it's ready to review/merge ? (cc @julien-c @LysandreJik as well)

If everyone is happy with the suggested approach, then I would suggest adding this updated __repr__ to all the classes in hf_api.py. Once that is done this PR should be ready for final review/to be merged.

@Wauplin
Copy link
Contributor

Wauplin commented Aug 12, 2022

I quickly looked at hf_api.py and as you suggested it makes sense to adapt the __repr__ for all of them (RepoFile ModelInfo, DatasetInfo, SpaceInfo and MetricInfo). I think it would also be good to group everything in a single abstract class (implementing only __repr__) and give it by inheritance instead of the current duplicated code.

@davanstrien Since you already started working on it, would you mind taking care of it ? Thanks in advance :)

@davanstrien
Copy link
Member Author

I quickly looked at hf_api.py and as you suggested it makes sense to adapt the __repr__ for all of them (RepoFile ModelInfo, DatasetInfo, SpaceInfo and MetricInfo). I think it would also be good to group everything in a single abstract class (implementing only __repr__) and give it by inheritance instead of the current duplicated code.

@davanstrien Since you already started working on it, would you mind taking care of it ? Thanks in advance :)

Sure, I'm super busy at the moment so I likely won't be able to look at this until mid-September.

@Wauplin
Copy link
Contributor

Wauplin commented Aug 24, 2022

Ok thanks, that's fine 👍

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Feb 27, 2023

The documentation is not available anymore as the PR was closed or merged.

@davanstrien davanstrien marked this pull request as ready for review February 28, 2023 10:05
@davanstrien
Copy link
Member Author

@Wauplin I've used a Mixin to implement this new __repr__ format. Let me know if you have something else in mind :)
I don't think the failing tests are because of these changes but let me know if I need to do anything to fix these.

Copy link
Contributor

@Wauplin Wauplin 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 completing this @davanstrien ! Failing tests are indeed not related. I'm merging the PR :)

@Wauplin Wauplin merged commit 5fb815c into huggingface:main Feb 28, 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.

Consider implementing Rich Repr Protocol for classes in hf_api
4 participants