poyo -> pyyaml (Issue #1513)

[acturner]

I understand it may not be desirable to add pyyaml as a dependency, but these few changes do address both Issue #1513 and Issue #1514.

[simobasso]

Hey, @acturner, please check #1516.

Sadly pyyaml can't be introduced without dropping windows users, please have a look at #1498 (comment)

[acturner]

@simobasso - Understood, unfortunately #1516 doesn't seem to fix these issues.
EDIT: #1516 fixes #1514 but not #1514

[simobasso]

@acturner, thanks for reaching out. I just tried with your fork, and it works. Did you change anything?

[acturner]

@simobasso Sorry I was mistaken, #1516 fixes #1514 - I had a similar solution, which doesn't require calling merge_configs when the key is missing (you can see in the diff). As far as fixing #1513, I just replaced poyo with pyyaml, which I know you said we can't use. It appears to be a problem with poyo ...

[samj1912]

Hey, @acturner, please check #1516.

Sadly pyyaml can't be introduced without dropping windows users, please have a look at #1498 (comment)

Hmm, AFAIK this should not be the case anymore. These yaml libraries now ship with wheels for windows so it should be possible to switch over. It is worth exploring moving to a better supported yaml library IMHO.

[glumia]

Please remove also lines 179:183 in tests/conftest.py, they are not needed anymore if we don't use poyo.

[acturner]

@glumia I believe I made all of your changes - let me know if anything doesn't look right. Cheers!

[glumia]

🔝 lgtm!

[acturner]

Updated title to reflect that this PR no longer fixes Issue #1514 (but PR #1516 does).

[simobasso]

Hey @acturner, thanks for this pr; I just enabled the ci.

Could you please check windows failures? Also, have you seen #1489 opened by @dHannasch?

[samj1912]

#1489 does seem like a similar implementation. It also fixes the test issues related to Windows builds.

[acturner]

@simobasso @samj1912 I will check out #1489 soon and see what's going on with the Windows builds - also should probably add some tests for the bug this fixes (in case anyone makes a PR to use poyo again or something that breaks similarly)

[ssbarnea]

I do support this change but I fixing windows builds is required before being able to merge it.

[ssbarnea]

Merged #1489 which did pass the tests.