Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Both PyYAML and oyaml in requirements #144

Closed
vreuter opened this issue Jan 25, 2024 · 5 comments
Closed

Both PyYAML and oyaml in requirements #144

vreuter opened this issue Jan 25, 2024 · 5 comments
Labels
enhancement New feature or request likely-solved
Milestone

Comments

@vreuter
Copy link
Member

vreuter commented Jan 25, 2024

oyaml is billed as a drop-in replacement for PyYAML, but both are listed in the requirements. Would just one suffice? If both are needed, could a note be added to the requirements declaration about why that is?

@vreuter vreuter added the question Further information is requested label Jan 25, 2024
donaldcampbelljr added a commit that referenced this issue Jan 25, 2024
@donaldcampbelljr
Copy link
Contributor

Excellent. Thanks for the catch. You are correct; it was redundant. I've made the changes to Dev.

@nsheff
Copy link
Contributor

nsheff commented Jan 26, 2024

is oyaml still needed or does pyyaml preserve ordering now that dicts are ordered by default?

  • oyaml is a single-file library with only 53 lines of code
  • oyaml is a drop-in replacement for PyYAML, which will load maps into collections.OrderedDict instead of regular dicts

See here:

https://stackoverflow.com/questions/13297744/pyyaml-control-ordering-of-items-called-by-yaml-load

But OrderedDict should no longer be necessary.

@nsheff
Copy link
Contributor

nsheff commented Jan 26, 2024

In Python 3.7, the items-ordered feature of dict objects was declared an official part of the Python language specification. So, from that point on, developers could rely on dict when they needed a dictionary that keeps its items ordered.

https://realpython.com/python-ordereddict/

As long as we're not supporting python 3.6...

@nsheff
Copy link
Contributor

nsheff commented Mar 12, 2024

It looks like we're still relying on oyaml, so this is not solved.

@nsheff nsheff added enhancement New feature or request and removed question Further information is requested likely-solved labels Mar 12, 2024
@nsheff nsheff added this to the v0.9.0 milestone Mar 12, 2024
@donaldcampbelljr
Copy link
Contributor

Ok, this should now be completed. I removed oyaml in favor of pyyaml per the above reading.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request likely-solved
Projects
None yet
Development

No branches or pull requests

3 participants