Skip to content

Pin PyYAML and add a test for a trailing tab#315

Merged
GraemeWatt merged 4 commits into
masterfrom
pin-pyyaml
Feb 24, 2021
Merged

Pin PyYAML and add a test for a trailing tab#315
GraemeWatt merged 4 commits into
masterfrom
pin-pyyaml

Conversation

@GraemeWatt
Copy link
Copy Markdown
Member

Closes #314.

@GraemeWatt GraemeWatt added type: bug Indicates an unexpected problem or unintended behaviour priority: low complexity: low labels Feb 23, 2021
@GraemeWatt GraemeWatt self-assigned this Feb 23, 2021
@coveralls
Copy link
Copy Markdown

coveralls commented Feb 23, 2021

Coverage Status

Coverage decreased (-0.01%) to 85.042% when pulling 961434a on pin-pyyaml into 481b4aa on master.

Comment thread tests/pyyaml_test.py Outdated
Comment on lines +28 to +29
except ImportError: # pragma: no cover
from yaml import SafeLoader as Loader # pragma: no cover
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does the test pass if SafeLoader is used instead of CSafeLoader? Perhaps we should check both - or if SafeLoader doesn't work, should we reconsider whether we should use it in the main code?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks for the comment. I've added a test that SafeLoader raises a yaml.scanner.ScannerError. This bug might be fixed in a future PyYAML version, then the test would fail if we upgraded the PyYAML version in requirements.txt. But now that PyYAML v5.4.1 includes the LibYAML extension in the Python wheel, I don't see a reason to use pure-Python PyYAML (which is also slower), so I've removed usage of SafeLoader etc. throughout the code.

@GraemeWatt GraemeWatt merged commit 66c1fc0 into master Feb 24, 2021
@GraemeWatt GraemeWatt deleted the pin-pyyaml branch February 24, 2021 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

complexity: low priority: low type: bug Indicates an unexpected problem or unintended behaviour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

requirements: pin PyYAML and add a test for a trailing tab

3 participants