Skip to content
New issue

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

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

Already on GitHub? # to your account

At least some boolean arguments in query functions are not working correctly #1146

Closed
buhrmann opened this issue Nov 2, 2022 · 1 comment · Fixed by #1152
Closed

At least some boolean arguments in query functions are not working correctly #1146

buhrmann opened this issue Nov 2, 2022 · 1 comment · Fixed by #1152
Assignees
Labels
bug Something isn't working

Comments

@buhrmann
Copy link

buhrmann commented Nov 2, 2022

Describe the bug

Functions like list_models have boolean arguments supposedly determining whether or not to include certain data in the results, e.g. full, cardData, or fetch_config. The mix of camel case and snake case aside, being type hinted as boolean, one would expect this to toggle the respective feature using True/False. Instead, one has to pass None instead of False to disable it.

This smells of a possible misunderstanding of what cardData: typing.Optional[bool] = None means, in that typing.Optional doesn't in fact make the argument optional, it simply allows None being explicitly passed as a value. It is made optional simply by providing a default value, and so I suspect that a better way to define these signatures is simply using cardData: bool = False.

Reproduction

Setting cardData=False doesn't turn this option off (only setting it to None does):

list_models(author="huggingface", cardData=False)[0]
ModelInfo: {
	modelId: huggingface/CodeBERTa-small-v1
	sha: None
	lastModified: None
	tags: []
	pipeline_tag: fill-mask
	siblings: None
	private: False
	author: None
	config: None
	_id: 621ffdc136468d709f17bd02
	id: huggingface/CodeBERTa-small-v1
	cardData: {'language': 'code', 'thumbnail': 'https://cdn-media.huggingface.co/CodeBERTa/CodeBERTa.png', 'datasets': ['code_search_net']}
}

Setting fetch_config=False doesn't turn this option off (only setting it to None does):

list_models(author="huggingface", fetch_config=False)[0]
ModelInfo: {
	modelId: huggingface/CodeBERTa-small-v1
	sha: None
	lastModified: None
	tags: []
	pipeline_tag: fill-mask
	siblings: None
	private: False
	author: None
	config: {'architectures': ['RobertaForMaskedLM'], 'model_type': 'roberta'}
	_id: 621ffdc136468d709f17bd02
	id: huggingface/CodeBERTa-small-v1
}

Logs

No response

System Info

huggingface_hub=0.10.1
@buhrmann buhrmann added the bug Something isn't working label Nov 2, 2022
@Wauplin
Copy link
Contributor

Wauplin commented Nov 3, 2022

Hi @buhrmann thanks for reporting ! That is definitely a bug and it seems to come from a server-side issue.

About the Optional[bool] / bool, I get your point. The slight difference between fetch_config=None and fetch_config=False is that in the first case the default behavior is decided by the server as in the second case it is hard-coded in the lib. This rule could be revisited but at least that's the short explanation about it.

EDIT: issue is internally tracked in https://github.com/huggingface/moon-landing/issues/4372.

EDIT 2: Will be solved from client-side. Behavior of the server is "if 'config' param is passed, return the config, no matter the value". So we will just stop sending 'config' if False or None.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants