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

Alternate YAMLSpecification load classmethod #197

Closed
ben-bay opened this issue Sep 24, 2019 · 3 comments
Closed

Alternate YAMLSpecification load classmethod #197

ben-bay opened this issue Sep 24, 2019 · 3 comments
Assignees

Comments

@ben-bay
Copy link
Contributor

ben-bay commented Sep 24, 2019

It would be useful if YAMLSpecification had a class method that accepted yaml in string form (instead of a path to a yaml file), as follows:

@classmethod
def load_spec_from_str(cls, string):
    # loads python objects from the yaml string, returns a YAMLSpecification

If that method existed, load_specification could just call it:

@classmethod
def load_specification(cls, path):
    with open(path, "r") as f:
        return YAMLSpecification.load_spec_from_str(f.read())
@FrankD412 FrankD412 self-assigned this Sep 25, 2019
@FrankD412
Copy link
Member

So, l was looking at loading from a string as specified above. It turns out that the yaml module does not have a loads variant of their parsing API (in a fashion akin to json which has a load for stream objects and loads for string objects). Considering most libraries will support stream objects (like file pointers) by default, I almost wonder if providing an API that takes in a stream is more flexible than limiting to a string. With the structure mentioned in your post, I would have to initialize a StringIO object from the string. If I can just assume a stream object from the start, it makes the API more flexible because it means I don't need to add APIs for all types of classes. At that point, it becomes the responsibility of the caller to conform streams.

@ben-bay, does that sound reasonable? It would mean that instead of me taking the string, you would just initialize the stream on your end and pass it to Maestro.

@ben-bay
Copy link
Contributor Author

ben-bay commented Sep 25, 2019

@FrankD412 that sounds reasonable indeed. It hadn't occurred to me to use streams, but this does seem like the correct place to use them.

@FrankD412
Copy link
Member

@ben-bay -- Merged. Feel free to pull from develop and use the feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants