Uploaded image for project: 'Picard'
  1. Picard
  2. PICARD-1406

Refactor Metadata.set and metadata.__setitem__

    XMLWordPrintable

    Details

    • Type: Task
    • Status: Closed
    • Priority: Normal
    • Resolution: Fixed
    • Affects Version/s: 2.0.4
    • Fix Version/s: 2.3.0
    • Component/s: Tags & Metadata
    • Labels:
      None

      Description

      I think from an API side it is actually pretty bad that we have both metadata[t] = v and metadata.set(t, v) which look so much alike but behave absolutely different. Furthermore the special behavior of metadata.set (namely allowing arbitrary values to be set, including empty, without str conversion) is currently not used by core Picard nor the existing plugins. There are some instances of metadata.set calls, but they mainly ensure data is copied 1:1, but themselves don't cause e.g. empty values to be added.

      Also the reading methods metadata.get(t) and metadata[t] behave exactly the same, so its inconsistent to have the setters behave different.

      But I was not brave enough to change it. Actually it was my first approach to unify both methods, but it is an API breaking change after all. What are your opinions?

      I personally would refactor the Metadata class as follows:

      1. Make set and _setitem_ be the same
      2. Make add and add_unique also do the str conversion for consistency
      3. Optional: If needed introduce some functions setraw and getraw to really set and get raw values without any conversion

       

      See discussion at https://github.com/metabrainz/picard/pull/1027

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                outsidecontext Philipp Wolfer
                Reporter:
                outsidecontext Philipp Wolfer
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Packages

                  Version Package
                  2.3.0