Opened 15 years ago
Closed 12 years ago
#12914 closed Cleanup/optimization (fixed)
Use yaml faster C implementation when available
Reported by: | Beuc | Owned by: | nobody |
---|---|---|---|
Component: | Core (Serialization) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
pyyaml does not use the C implementation automatically.
Instead, it uses the Python implementation which is at least twice as slow.
This is one of the first thing that the documentation covers:
http://pyyaml.org/wiki/PyYAMLDocumentation#Installation
The attached patch uses the C implementation when available.
It would be nice to add a warning when the Python implementation is used, so that users are encouraged to install libyaml, but I'm not sure how to do it properly.
Attachments (2)
Change History (17)
by , 15 years ago
Attachment: | pyyaml.py-svn-modified added |
---|
comment:1 by , 15 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 15 years ago
milestone: | 1.2 |
---|
1.2 is feature-frozen, moving this feature request off the milestone.
comment:3 by , 15 years ago
This is more a bug-fix than a feature IMHO.
I'd recommend including it in 1.2.
comment:4 by , 14 years ago
Type: | → Cleanup/optimization |
---|
comment:5 by , 14 years ago
Severity: | → Normal |
---|
comment:6 by , 14 years ago
Easy pickings: | unset |
---|---|
Patch needs improvement: | set |
pyyaml.patch fails to apply cleanly on to trunk
comment:8 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:9 by , 12 years ago
Resolution: | fixed |
---|---|
Status: | closed → new |
The commit is causing some failures in timezone tests (there might be other failures, too, haven't ran the full suite):
(py3_venv)akaariai@akaariai-UX31E:~/Programming/django/tests$ ./runtests.py --settings=test_sqlite timezones Creating test database for alias 'default'... Creating test database for alias 'other'... .......sssssss....s..............s......s...............FFFFFF................. ====================================================================== FAIL: test_aware_datetime_in_local_timezone (modeltests.timezones.tests.SerializationTests) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/akaariai/Programming/django/tests/modeltests/timezones/tests.py", line 625, in test_aware_datetime_in_local_timezone self.assert_yaml_contains_datetime(data, "2011-09-01 13:20:30+03:00") File "/home/akaariai/Programming/django/tests/modeltests/timezones/tests.py", line 507, in assert_yaml_contains_datetime self.assertIn("- fields: {dt: !!timestamp '%s'}" % dt, yaml) AssertionError: "- fields: {dt: !!timestamp '2011-09-01 13:20:30+03:00'}" not found in "- fields: {dt: ! '2011-09-01 13:20:30+03:00'}\n model: timezones.event\n pk: null\n" <SNIP similar errors...> ---------------------------------------------------------------------- Ran 79 tests in 0.332s FAILED (failures=6, skipped=10)
The failures happen only if pyyaml is installed, and only on Python 3. Here is freeze of the py3_venv:
Markdown==2.2.1 PyYAML==3.10 cx-Oracle==5.1.2 distribute==0.6.30 djangobench==0.9 docutils==0.10 flake8==1.7.0 ipdb==0.7 ipython==0.13.1 psycopg2==2.4.6 pytz==2012j textile==2.1.5 wsgiref==0.1.2
comment:10 by , 12 years ago
Has patch: | unset |
---|---|
Patch needs improvement: | unset |
Severity: | Normal → Release blocker |
comment:11 by , 12 years ago
Aymeric, this was not committed in 1.5. Did you still want to set this as a blocker?
I can also reproduce it with Python 2. Apparently, this is only a difference in representation, and loading both formats seems to give back the appropriate datetime object.
So this might be simply solved by changing the test assertion
- self.assertIn("- fields: {dt: !!timestamp '%s'}" % dt, yaml) + # Depending on the yaml dumper, '!timestamp' might be absent + self.assertRegexpMatches(yaml, + r"- fields: {dt: !(!timestamp)? '%s'}" % re.escape(dt))
comment:14 by , 12 years ago
PyYAML isn't very good with datetimes in general. I don't like changing tests because we can't get the output we want, but in this case, we don't have much choice.
PyYAML's documentation helpfully states that "there are some subtle (but not really significant) differences between pure Python and LibYAML based parsers and emitters" which may include the difference we're seeing here. I'm not sure it's worth reporting this as a bug.
Apparently PyYAML moved to bitbucket two weeks ago, after 18 months without any activity.
comment:15 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Use yaml faster C implementation when available