make Metadata a dataclass

[edsaac]

Currently, the Metadata class has no __init__ function but only class variables.

class Metadata:
    """Custom class for metadata.
    """
    url = None
    query_time = None
    site_info = None
    header = None
    variable_info = None
    comment = None

    # note sure what statistic_info is
    statistic_info = None
    # disclaimer seems to be only part of importWaterML1
    disclaimer = None

I think these are not meant to be class variables but instance variables, as different data requests will return different metadata. Check an example of the difference in https://docs.python.org/3/tutorial/classes.html#class-and-instance-variables. Only utils.set_metadata creates Metadata objects and assigns values to the instance object variables, without ever modifying the default None of the class variables. nwis._set_metadata behaves similarly in that it only modify objects but never the class itself.

Making the Metadata a dataclass helps with making those (url, site_info, etc) instance attributes, without the need to write a long __init__ function. Dataclasses have been part of the standard library since 3.7 so there should not be compatibility issues.

---

• All tests passing from the test folder using pytest.

[elbeejay]

Thanks for the contribution and detailed explanation @edsaac - I agree that as implemented the Metadata class does not follow best practices.

This relates to #75, and is a good solution I think. Will let @thodson-usgs weigh in too as he opened #75 and likely has some thoughts on the Metadata class.

[elbeejay]

Do we need this at the end of the script? We don't have any handling for direct calls of any other other .py files.

[edsaac]

Not really, that idiom is not really necessary unless the file was run as a script.

[elbeejay]

This works for me but @thodson-usgs it'd be great to get your approval too before we merge.

[thodson-usgs]

I agree that existing implementation was poor. That said, I'm not sure changing to a dataclass really fixes anything.

It may be better to use an init function here to abstract how the Metadata is initialized. Something like Metadata(response) seems ideal, but I haven't thought through all the implications yet

[edsaac]

I agree, making it a dataclass just removes those class variables that are initialized to None and never used. But it does not bring any big improvement to the rest of the code. Constructing the object with Metadata(response) makes much more sense, so I will close this PR and give it a try to that idea.

[thodson-usgs]

Thanks @edsaac,
I'm a preoccupied with another deadline, but if you're interested, you could proceed like this:
Submit a PR that implements a basic Metadata class, then you and I can work together on that PR to create children classes for the different services.