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

Implement from_object/from_string method in Metadata class #1336

Closed
sechkova opened this issue Apr 7, 2021 · 4 comments · Fixed by #1354
Closed

Implement from_object/from_string method in Metadata class #1336

sechkova opened this issue Apr 7, 2021 · 4 comments · Fixed by #1354
Assignees
Labels
enhancement good first issue Bite-sized items for first time contributors

Comments

@sechkova
Copy link
Contributor

sechkova commented Apr 7, 2021

Description of feature request:
Implement a class method in Metadata class, similar to the existing from_file method but taking a file object as an argument.

See #1304 for context.

@sechkova sechkova added the experimental-client Items related to the development of a new client (see milestone/8 and theexperimental-client branch) label Apr 7, 2021
@sechkova sechkova added this to the Client Refactor milestone Apr 7, 2021
@jku
Copy link
Member

jku commented Apr 7, 2021

I assume this is useful because client does not write new metadata to disk before it's verified so at that point there's only a file-like object but we want to instantiate the Metadata to verify it is correct?

I guess the other alternative to from_object() is from_string() (and client would then call from_string(tmp_file.read())). I don't think it's clearly better just thinking out loud.

@sechkova
Copy link
Contributor Author

sechkova commented Apr 7, 2021

I assume this is useful because client does not write new metadata to disk before it's verified so at that point there's only a file-like object but we want to instantiate the Metadata to verify it is correct?

Yes, exactly.

I guess the other alternative to from_object() is from_string() (and client would then call from_string(tmp_file.read())). I don't think it's clearly better just thinking out loud.

You don't like a function signature with the vague definition of a file object? :)
String would be ok, raw bytes too, no need to be a file object with its associated methods.

Now that a separate serialisation was implemented, all we need is basically a method that calls deserialize.

@sechkova
Copy link
Contributor Author

sechkova commented Apr 7, 2021

Something like the existing from_file but without the actual file read from the file system:

  @classmethod
   def from_string/from_bytes/from_raw(
       cls,
       raw_data: str/bytes,
       deserializer: Optional[MetadataDeserializer] = None
   ) -> "Metadata":

       if deserializer is None:
           deserializer = JSONDeserializer()

       return deserializer.deserialize(raw_data)

@joshuagl joshuagl added good first issue Bite-sized items for first time contributors and removed experimental-client Items related to the development of a new client (see milestone/8 and theexperimental-client branch) labels Apr 13, 2021
@jku
Copy link
Member

jku commented Apr 16, 2021

looks good to me -- str seems to make sense as type to me

@jku jku changed the title Implement from_object method in Metadata class Implement from_object/from_string method in Metadata class Apr 16, 2021
@jku jku self-assigned this Apr 16, 2021
@jku jku closed this as completed in #1354 Apr 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement good first issue Bite-sized items for first time contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants