Skip to content

[python] Added a draft of a transformer into native python objects.#169

Open
KOLANICH wants to merge 1 commit intobiojppm:masterfrom
KOLANICH-libs:python_transformer
Open

[python] Added a draft of a transformer into native python objects.#169
KOLANICH wants to merge 1 commit intobiojppm:masterfrom
KOLANICH-libs:python_transformer

Conversation

@KOLANICH
Copy link
Copy Markdown

@KOLANICH KOLANICH commented Nov 25, 2021

Currently IDK how to identify a type of a value, so all the values are strings.

@biojppm
Copy link
Copy Markdown
Owner

biojppm commented Nov 25, 2021

Thanks for the MR - it is definitely useful. Can you add unit tests?

@KOLANICH
Copy link
Copy Markdown
Author

Probably. Should I add them into an already existing file or should I create a new one?

@biojppm
Copy link
Copy Markdown
Owner

biojppm commented Nov 25, 2021

You can create a new one, in this folder

@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 29, 2021

Codecov Report

Merging #169 (b817bd7) into master (74c8031) will decrease coverage by 0.04%.
The diff coverage is n/a.

❗ Current head b817bd7 differs from pull request most recent head feb7501. Consider uploading reports for the commit feb7501 to get more accurate results

@@            Coverage Diff             @@
##           master     #169      +/-   ##
==========================================
- Coverage   96.39%   96.34%   -0.05%     
==========================================
  Files          22       63      +41     
  Lines        8064    12781    +4717     
==========================================
+ Hits         7773    12314    +4541     
- Misses        291      467     +176     
Impacted Files Coverage Δ
src/c4/yml/common.hpp 90.47% <0.00%> (-9.53%) ⬇️
src/c4/yml/parse.cpp 92.71% <0.00%> (-3.05%) ⬇️
src/c4/yml/common.cpp 85.41% <0.00%> (-2.96%) ⬇️
src/c4/yml/parse.hpp 98.55% <0.00%> (-1.45%) ⬇️
src/c4/yml/node.hpp 97.65% <0.00%> (-0.20%) ⬇️
src/c4/yml/detail/print.hpp 95.83% <0.00%> (-0.17%) ⬇️
src/c4/yml/tree.hpp 98.85% <0.00%> (-0.12%) ⬇️
src/c4/yml/tree.cpp 94.45% <0.00%> (-0.07%) ⬇️
samples/quickstart.cpp 99.50% <0.00%> (-0.04%) ⬇️
src/c4/yml/node.cpp 100.00% <0.00%> (ø)
... and 53 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@biojppm
Copy link
Copy Markdown
Owner

biojppm commented Nov 29, 2021

What's up with this error?

@KOLANICH
Copy link
Copy Markdown
Author

KOLANICH commented Nov 29, 2021

It is extremily strange, especially given that my machine is 64-bit Ubuntu (for which the test pipeline also fails, it looks like it fails for all 64-bit machines, but for me the test works).

BTW what should I do to the DOC node (present when the root is a scalar, these cases are commented out)? And is there any way to get a type (string, int, float, bool) of a value node?

@KOLANICH KOLANICH force-pushed the python_transformer branch 3 times, most recently from e31d4cb to a0cacb9 Compare May 11, 2022 15:50
@KOLANICH
Copy link
Copy Markdown
Author

@biojppm

@KOLANICH
Copy link
Copy Markdown
Author

ruamel.yaml safe parser is currently faster than ryml-based parser with my very rudimentary postprocessor.

2000 files:
ruamel.yaml: 23.7 s;
ryaml.parse_to_python: 20.5 s
ryaml.parse_in_arena: 0.5 s

1 file bigger and more complex than any of them (100 iterations)
ruamel.yaml: 18.0 s;
ryaml.parse_to_python: 22.3 s
ryaml.parse_in_arena: 0.45 s

So, if accelerate yaml parsing for python using ryaml, then the conversion should probably be done in native code.

@KOLANICH KOLANICH force-pushed the python_transformer branch 4 times, most recently from 4f401a1 to 032524d Compare May 18, 2022 09:06
@biojppm biojppm force-pushed the master branch 4 times, most recently from d128813 to 507400e Compare April 16, 2024 23:52
@biojppm biojppm force-pushed the master branch 2 times, most recently from fbee0da to e4aa0b3 Compare August 24, 2025 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants