Skip to content

fix a bug in the .readthedocs.yml file#61

Merged
ahms5 merged 5 commits intomainfrom
fix-.readthedocs
Apr 16, 2026
Merged

fix a bug in the .readthedocs.yml file#61
ahms5 merged 5 commits intomainfrom
fix-.readthedocs

Conversation

@h-chmeruk
Copy link
Copy Markdown
Contributor

The test for Read the Docs failed in the PR#927 of the pyfar package because the python version in the tools in the .readthedocs.yml file was not enclosed in quotation marks.

Changes proposed in this pull request:

  • Enclose {{ latest_python_version }} in the quotation marks
  • Adjust the test function in the test_copier.py file

@h-chmeruk h-chmeruk added this to the v0.1.0 milestone Apr 13, 2026
@h-chmeruk h-chmeruk added the bug Something isn't working label Apr 13, 2026
@h-chmeruk h-chmeruk requested a review from ahms5 April 13, 2026 07:49
@h-chmeruk h-chmeruk self-assigned this Apr 13, 2026
Copy link
Copy Markdown
Member

@ahms5 ahms5 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 catching, not sure of the quotes are the issue. see below:

Comment thread template/.readthedocs.yml.jinja Outdated
Comment on lines 13 to 15
{% if 'libsndfile1' in apt_packages -%}
apt_packages:
- libsndfile1
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
{% if 'libsndfile1' in apt_packages -%}
apt_packages:
- libsndfile1
{% if 'libsndfile1' in apt_packages -%}
apt_packages:
- libsndfile1

I think the bug is the intent of apt_packages: it does not have any spaces in front, but two are requried.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh, indeed, thank you for pointing that out!
I think the absence of quotation marks was also a major issue, at least that’s what the error message on ReadTheDocs said...

@h-chmeruk h-chmeruk requested a review from ahms5 April 15, 2026 11:05
Copy link
Copy Markdown
Member

@ahms5 ahms5 left a comment

Choose a reason for hiding this comment

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

Thank you, I just noticed an other point, sorry for not mentioning it earlier,

Comment thread tests/test_copier.py Outdated
@pytest.mark.parametrize("desired", [
'- libsndfile1',
"python: 3.14",
'python: "3.14"',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

maybe also add this in order to prevent that error in future.

Suggested change
'python: "3.14"',
'python: "3.14"',
' apt_packages:',

{% if 'libsndfile1' in apt_packages -%}
python: "{{ latest_python_version }}"
{% if 'libsndfile1' in apt_packages -%}
apt_packages:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry, I did not see it earlier, I think we can loop through all apt_packages here in the list below. In this way every package in apt_packages will be installed, which is also the intended use.

@h-chmeruk h-chmeruk requested a review from ahms5 April 16, 2026 09:13
Copy link
Copy Markdown
Member

@ahms5 ahms5 left a comment

Choose a reason for hiding this comment

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

almost there, thank you, I think we should add a test for empty apt_packages. what do you think?

@h-chmeruk
Copy link
Copy Markdown
Contributor Author

@ahms5, yeah, sounds good, I'll add it

@h-chmeruk h-chmeruk requested a review from ahms5 April 16, 2026 12:06
@ahms5 ahms5 merged commit be62471 into main Apr 16, 2026
11 checks passed
@github-project-automation github-project-automation bot moved this from Require review to Done in Weekly Planning Apr 16, 2026
@ahms5 ahms5 deleted the fix-.readthedocs branch April 16, 2026 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants