Migrate several classes over to dataclasses

[eli-schwartz]

Python 3.7 has been the minimum requirement for a while now, which brings dataclasses into the standard library. This reduces the boilerplate necessary to write nice classes with init and repr methods that simply do the right thing.

As a side effect, fix the repr of installer.scripts.Script, which was missing the trailing close-parenthesis:

>>> import installer
>>> installer.scripts.Script('name', 'module', 'attr', 'section')
Script(name='name', module='module', attr='attr'

[eli-schwartz]

There are a couple improvements I'd like to make to Script / SchemeDictionaryDestination for downstream use in projg2/gpep517#13

Merging this PR would help make it easier to add those changes, for all the usual reasons...

[pradyunsg]

This has been on my radar for a bit, but I haven't had much free time lately.

[edgarrmondragon]

Probably not a big deal but for some reason, sphinx-autodoc is placing some dataclass attributes after the __init__ method:

[eli-schwartz]

I don't know anything about why sphinx might choose to do this.

[eli-schwartz]

Were there any other outstanding review requests? I think I got everything?

[eli-schwartz]

Cleanly rebased against upstream -- no changes were needed.

[eli-schwartz]

Ping.

[eli-schwartz]

Rebased to fix conflicts (again).

1:  37b4ba2 ! 1:  f0f0da0 Migrate several classes over to dataclasses
    @@ src/installer/records.py: __all__ = [
     +    issues: Iterable[str]
      
     -    def __repr__(self) -> str:
    --        return "InvalidRecordEntry(elements={!r}, issues={!r})".format(
    --            self.elements, self.issues
    --        )
    +-        return f"InvalidRecordEntry(elements={self.elements!r}, issues={self.issues!r})"
     +    def __post_init__(self) -> None:
     +        super().__init__(", ".join(self.issues))
      
    @@ src/installer/scripts.py: class InvalidScript(ValueError):
     -        self.section = section
     -
     -    def __repr__(self) -> str:
    --        return "Script(name={!r}, module={!r}, attr={!r}".format(
    --            self.name,
    --            self.module,
    --            self.attr,
    --        )
    +-        return f"Script(name={self.name!r}, module={self.module!r}, attr={self.attr!r}"
     +    section: "ScriptSection" = field(repr=False)
     +    """
     +    Denotes the "entry point section" where this was specified. Valid values

New code hasn't changed, but the code I deleted had gotten reformatted in the meantime.

[eli-schwartz]

@pradyunsg any update on a review for this?

I don't mean to be a nag. I'm fiercely supportive of the rights of project developers to guide their projects as they see fit, and implement what they see fit to implement. Nobody owes anyone anything here, we are all just internet strangers posting code we liked, that we occasionally hope others find useful as well.

But... I did open this PR back in November, and have rebased it numerous times to fix conflicts. I need it because I'm trying to build my own tooling using this library and this PR is a blocker for my work. You still don't owe me anything! It is just that at some point I'd like to resume my work, and I need to know whether I should wait for a new release of installer, or gratefully take your code and adapt it to fit my needs. I also need to be able to confirm that status to the people I'm collaborating with myself. ;)

It's the "not knowing" that is killing me here.

I know you said back in November that you hadn't had a lot of free time recently. There have been 118 commits to git main since then, although admittedly the majority were from pre-commit.ci or related tooling, so my working theory/hope was that this is still just more of the age-old story of free open-source volunteers not having time to obey the whim of every random stranger on the internet asking favors.

(I would almost suggest that I would be willing to put my money where my mouth is and help out as a comaintainer. As a member of the Gentoo Python team I have a vested long-term interest in build backend/frontend technology including this, and I want to see installer flourish. But while there are certainly people who know me very well, you don't have a clue who I am, and well, Jia Tan did a lot of damage there. 😢)

...

Meanwhile, the most recent lint PR has once again reformatted lines that my PR would simply delete. I could rebase to fix conflicts again, but at this point I would prefer to wait until I only have to do so a single time shortly before merging.

[Secrus]

@eli-schwartz Hi. Sorry for the wait on this PR. I will take care of this PR now and hopefully, we will get this merged by the end of the week. Feel free to rebase and ping me once you do, I will review it.

[eli-schwartz]

@Secrus thanks for offering to look into this.

I've done the rebase. Here is a range-diff of the conflict resolution I had to perform (once to resolve the moved noqa D107 in 76d1957 by still deleting it, and once to incorporate the newly added API from 0b003fa):

1:  f0f0da0 ! 1:  819f6e1 Migrate several classes over to dataclasses
    @@ Commit message
     
      ## src/installer/destinations.py ##
     @@
    - import compileall
    + 
      import io
      import os
     +from dataclasses import dataclass
    @@ src/installer/destinations.py: class WheelDestination:
     -        hash_algorithm: str = "sha256",
     -        bytecode_optimization_levels: Collection[int] = (),
     -        destdir: Optional[str] = None,
    +-        overwrite_existing: bool = False,
     -    ) -> None:
     -        """Construct a ``SchemeDictionaryDestination`` object.
     -
    @@ src/installer/destinations.py: class WheelDestination:
     -        :param destdir: A staging directory in which to write all files. This
     -            is expected to be the filesystem root at runtime, so embedded paths
     -            will be written as though this was the root.
    +-        :param overwrite_existing: silently overwrite existing files.
     -        """
     -        self.scheme_dict = scheme_dict
     -        self.interpreter = interpreter
    @@ src/installer/destinations.py: class WheelDestination:
     -        self.hash_algorithm = hash_algorithm
     -        self.bytecode_optimization_levels = bytecode_optimization_levels
     -        self.destdir = destdir
    +-        self.overwrite_existing = overwrite_existing
     +    scheme_dict: Dict[str, str]
     +    """A mapping of {scheme: file-system-path}"""
     +
    @@ src/installer/destinations.py: class WheelDestination:
     +    filesystem root at runtime, so embedded paths will be written as though
     +    this was the root.
     +    """
    ++
    ++    overwrite_existing: bool = False
    ++    """Silently overwrite existing files."""
      
          def _path_with_destdir(self, scheme: Scheme, path: str) -> str:
              file = os.path.join(self.scheme_dict[scheme], path)
    @@ src/installer/records.py: __all__ = [
      class InvalidRecordEntry(Exception):
          """Raised when a RecordEntry is not valid, due to improper element values or count."""
      
    --    def __init__(
    +-    def __init__(  # noqa: D107
     -        self, elements: Iterable[str], issues: Iterable[str]
    --    ) -> None:  # noqa: D107
    +-    ) -> None:
     -        super().__init__(", ".join(issues))
     -        self.issues = issues
     -        self.elements = elements

[eli-schwartz]

Thanks. In a bit I will look into applying some of the implementation from my gpep517 feature branch as new API for installer.