huggingface / transformers

🤗 Transformers: State-of-the-art Machine Learning for Pytorch, TensorFlow, and JAX.
https://huggingface.co/transformers
Apache License 2.0
131.71k stars 26.22k forks source link

[doc] post-conversion: incosistent "default to" #14949

Closed stas00 closed 2 years ago

stas00 commented 2 years ago

After the conversion we ended up with inconsistent values for defaults to - sometimes it's formatted, other times it's italics, with former being the prevailing form. Example:

src/transformers/generation_tf_utils.py:            use_cache: (`bool`, *optional*, defaults to `True`):
src/transformers/generation_tf_utils.py:            output_attentions (`bool`, *optional*, defaults to *False*):

but there is a lot more of these

@sgugger

sgugger commented 2 years ago

It's not the conversion fault, those were inconsistent before. Meaning some were properly set with an :obj: marker or double backticks while other had simple backticks and were thus converted in italics.

By all means, if you want to do a batch conversion, the proper format is `False` (or `True`)

stas00 commented 2 years ago

Will do,

Also while fixing this I see there are still quite a few :obj: leftovers - is it still a WIP?

grep -Ir :obj: src
src/transformers/models/t5/modeling_t5.py:            assert self.is_decoder, f":obj:`use_cache` can only be set to `True` if {self} is used as a decoder"
src/transformers/models/tapas/modeling_tapas.py:        input_mask_float (:obj: *torch.FloatTensor* of shape `(batch_size, seq
[...]
sgugger commented 2 years ago

No. The first example is not a docstring, so not touched by the conversion scripts. The second one seemed to have a space between the :obj: and the ` so was not properly converted (the torch.FloatTensor is not supposed to be in italics).

You should wait a tiny bit more before doing any conversion as I'm in the process of changing lots of docstrings in #14950 (will merge it when it's green).

stas00 commented 2 years ago

That was just a small snippet, the full match is 44 lines. should be easy to reproduce.

Please take your time, I was just flagging these in case it was missed.

sgugger commented 2 years ago

OK PR is merged! If you want to hunt down all the remaining :obj:/:class:/:meth:/:func: that would be amazing (and leave me more time to work on #14032 this week :-) )

stas00 commented 2 years ago

Sure, once you approve https://github.com/huggingface/transformers/pull/14951 and it is merged to avoid conflicts.

sgugger commented 2 years ago

Done. I think there might also be some incomplete :obj or obj: in the wild (saw one on #14951).

stas00 commented 2 years ago

So what of all these :obj: that are not in the doc string - do we simply remove those?

e.g.:

src/transformers/models/t5/modeling_t5.py:            assert self.is_decoder, f":obj:`use_cache` can only be set to `True` if {self} is used as a decoder"
examples/research_projects/quantization-qdqbert/utils_qa.py:        prefix (:obj:`str`, `optional`):
examples/research_projects/deebert/src/modeling_highway_roberta.py:            loss (:obj:`torch.FloatTensor` of shape :obj:`(1,)`, `optional`, returned when :obj:`label` is provided):
sgugger commented 2 years ago

Yes, we use Markdown style everywhere. Would just not touch the research projects. as I haven't converted the docstrings there.