conflicting_files of DiscussionWithDetails is returned as a bool

[ademait]

Describe the bug

The conflicting_files parameter of DiscussionWithDetails object is returned as a bool when I call the get_discussion_details method. It happens with model oliverguhr/fullstop-punctuation-multilang-large in discussion number 6. The object (not full) is:

DiscussionWithDetails(title='Add multilingual to the language tag',
 status='closed',
 num=6, repo_id='oliverguhr/fullstop-punctuation-multilang-large',
 repo_type='model',
 author='lbourdois',
 is_pull_request=True, created_at=datetime.datetime(2023, 1, 7, 17, 4, 9, tzinfo=datetime.timezone.utc), 
 endpoint='https://huggingface.co' 
 
 ... 
 
 DiscussionStatusChange(id='63fca8e8b907d43a91d8a879',
 type='status-change',
 created_at=datetime.datetime(2023, 2, 27, 12, 58, 16, tzinfo=datetime.timezone.utc), author='oliverguhr',
 _event={'id': '63fca8e8b907d43a91d8a879',
 'author': {'avatarUrl': 'https://aeiljuispo.cloudimg.io/v7/https://cdn-uploads.huggingface.co/production/uploads/5ea841a70d1df220780f0433/rpEmnBmuIImkp2yo_pzfO.jpeg?w=200&h=200&f=face',
 'fullname': 'Oliver Guhr',
 'name': 'oliverguhr',
 'type': 'user',
 'isPro': False, 'isHf': False, 'isOwner': True, 'isOrgMember': False}, 'createdAt': '2023-02-27T12:58:16.000Z',
 'type': 'status-change',
 'data': {'status': 'closed'}}, new_status='closed')], conflicting_files=True, target_branch='refs/heads/main',
 merge_commit_oid=None, diff=None)

Reproduction

repo_name = 'oliverguhr/fullstop-punctuation-multilang-large'
details = api.get_discussion_details(repo_id=repo_name, discussion_num=6, repo_type='model, token=ACCESS_TOKEN)

Logs

Exception has occurred: TypeError
'bool' object is not iterable
...
TypeError: 'bool' object is not iterable

System info

- huggingface_hub version: 0.19.2
- Platform: Linux-6.2.16-4-pve-x86_64-with-glibc2.28
- Python version: 3.9.12
- Running in iPython ?: Yes
- iPython shell: ZMQInteractiveShell
- Running in notebook ?: Yes
- Running in Google Colab ?: No
- Token path ?: /root/.cache/huggingface/token
- Has saved token ?: False
- Configured git credential helpers: 
- FastAI: N/A
- Tensorflow: N/A
- Torch: N/A
- Jinja2: 3.1.2
- Graphviz: N/A
- Pydot: N/A
- Pillow: N/A
- hf_transfer: N/A
- gradio: N/A
- tensorboard: N/A
- numpy: N/A
- pydantic: N/A
- aiohttp: N/A
- ENDPOINT: https://huggingface.co
- HF_HUB_CACHE: /root/.cache/huggingface/hub
- HF_ASSETS_CACHE: /root/.cache/huggingface/assets
- HF_TOKEN_PATH: /root/.cache/huggingface/token
- HF_HUB_OFFLINE: False
- HF_HUB_DISABLE_TELEMETRY: False
- HF_HUB_DISABLE_PROGRESS_BARS: None
- HF_HUB_DISABLE_SYMLINKS_WARNING: False
- HF_HUB_DISABLE_EXPERIMENTAL_WARNING: False
- HF_HUB_DISABLE_IMPLICIT_TOKEN: False
- HF_HUB_ENABLE_HF_TRANSFER: False
- HF_HUB_ETAG_TIMEOUT: 10
- HF_HUB_DOWNLOAD_TIMEOUT: 10

[Wauplin]

Thanks for the report @ademait. I've looked into the backend and we indeed have a comment

/**
* The list of files that have conflicts
* true means there are conflicts but we can't list them due to an error
*/

Meaning that yes, a value True is expected in some cases (I didn't know about that 😄).
Would you mind opening a PR to fix the typing and docstring in huggingface_hub.DiscussionWithDetails?

[ademait]

Yes, I'll look at it over the next few days!

However, I think that returning a variable that can be either a bool or a list (i.e., not having a specific type) is not a good practice.

[Wauplin]

However, I think that returning a variable that can be either a bool or a list (i.e., not having a specific type) is not a good practice.

Yep, maybe not best but not problematic enough to consider changing the API right now either 🙂 Let's hope that if we type annotate it in the documentation and code, most issues will be avoided.

Btw, out of curiosity what is your use case for checking conflicting files in PRs? Are you building some projects around the Community Tab?

[ademait]

Hi @Wauplin , I agree.

Yes, I'm researcher, and I'm interested in open source, community management among others. We also try to underline the importance of social aspects of open source development. We are currently working on HFCommunity, providing easy and direct access to the Hub so researchers and everyone can leverage on. So that's way I'm digging into each library method 😄 I try to report everything I see.

[Wauplin]

Interesting! And yes, your feedback on the API in the last days is definitely much appreciated 🙏 🙏

[ademait]

For further feedback, should I open an issue for each report? Or do you prefer to keep the discussion in this thread?

[julien-c]

We are currently working on HFCommunity, providing easy and direct access to the Hub so researchers and everyone can leverage on.

Cool project!

[Wauplin]

For further feedback, should I open an issue for each report? Or do you prefer to keep the discussion in this thread?

I'd say it's preferable to open new issues when topics are different. It's easier for me to keep track of them. Thanks in advance!

[ademait]

Thank you @julien-c !

Okay @Wauplin , I'll open new threads then 😄 Also, we are working on a project which can provide insightful feedback to the Hub. Is there a channel where I can give you such info. Maybe by Discord, I'm already in.

[Wauplin]

@ademait Yes let's continue the chat on Discord if that's easier for feedback (my username: wauplinhf). Otherwise I'll be waiting for the new threads 😨 😄

[ademait]

Hi @Wauplin , I tried submitting the PR but I'm having some issues with the doc-builder. I've followed the steps of docs/README.md, installing the generated packages with:

pip install -e ".[docs]"

However, when generating the docs with:

doc-builder build huggingface_hub docs/source/en/ --build_dir ~/tmp/test-build

It throws: TypeError: There was an error when converting docs\source\en\package_reference\hf_api.md to the MDX format. expected string or bytes-like object. I only added a line in l:121 of file src/huggingface_hub/community.py. I added the last sentence:

conflicting_files (`list` of `str`, *optional*):
            A list of conflicting files if this is a Pull Request.
            `None` if `self.is_pull_request` is `False`.
            `True` if there are conflicting files but the list can't be retrieved. <-- THIS LINE

I tried a few times. I don't really know why it crashes. But as it is a minimal change I thought of reporting it directly here.

Cheers,

[Wauplin]

Hmm, this is a bit hard to help with. I don't see a reason why it would break the docs so could you try:

1. Push your code diff to a PR? This will trigger the CI and build the documentation. If a problem happens there, I'll be more able to investigate it.
2. (unrelated to 1.) Try to build documentation from the main branch. It is possible that the doc builder is broken for some reasons that is not related to your docstring update.

---

(nit) also about the change itself, it'd be nice to update the type in the docstring to

`conflicting_files (`Union[List[str], bool, None]`, *optional*):`

Thanks again for your help!

[ademait]

Hi @Wauplin , I created the PR, it's already linked.
Regarding the 2nd option, I had the same issues in the main branch. I don't know either why's it happening. I created a conda environment specifically for submitting these changes.

[Wauplin]

Thanks for the PR @ademait! Regarding the issue with building documentation locally, it is because the hf-doc-builder package is outdated. I've made a PR (#1849) to install the package from source when doing pip install -e ".[docs]" locally which should fixed the problem. FYI, I have been able to reproduce your error locally as well (and the PR fixed it) so definitely nothing related to your PR changes!

[ademait]

Great @Wauplin ! Thank you