#38 add metadata

[funkydunc]

#38

AddMetadata class with tests

[digitaldogsbody]

@funkydunc Can you rebase against main now that #63 is merged? That should hopefully fix the failing tests.

[digitaldogsbody]

I don't know why the linter is failing only for one Python version 😕

Given that the tests pass for the others, I don't see this as a block to merge, but it would be nice to work out why!

[azaroth42]

What happens if the function is passed a dict or langstring instance, per Glen's comment?

[funkydunc]

What happens if the function is passed a dict or langstring instance, per Glen's comment?

@azaroth42 @glenrobson how would you like the function to accept args as different datatypes? Would something like this, using additional keyword arguments on the method be ok?

    def add_metadata(self, label, value, metadata_dict=None, langstring=None):
        #check the presence of the above arguments (and they are valid) before adding them to the metadata

[glenrobson]

I'd lean more on others (cc @azaroth42 @digitaldogsbody @giacomomarchioro) on if this is the python way of doing things but could you check the type to see if its String, dict or LngString?

[azaroth42]

+1 to testing the type of the argument. if type(label) == dict: # do dict stuff

[funkydunc]

@azaroth42 @glenrobson is there an exception type you would want me to import and raise if the supplied arguments are invalid? Thanks.

[digitaldogsbody]

I would say raise TypeError if the value is not one of the correct types

[azaroth42]

If the param is invalid, then upstream validation in pydantic should catch it? I would be fine with handling valid cases and ignoring invalid ones.

[funkydunc]

@azaroth42 The add_metadata function now also accepts dict and LngString parameters.

It validates that the label and value params are either str, LngString or dict and, that they are the same type as each other.

Incorrect types, or params of unequal types returns ValueError. Validation also has a test.

Tests pass locally, although cancelled here.

[glenrobson]

We need to re-run the tests to see why it failed.

Mike to change workflow on: target - forces workflow to run from main rather than remote target.