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.

Sign up for GitHub

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

Jump to bottom

Replace partial applications of Dataset with Dataset subclasses #1145

Merged
merged 6 commits into from
Aug 10, 2023
Merged

Replace partial applications of Dataset with Dataset subclasses #1145

merged 6 commits into from
Aug 10, 2023

Commits on Aug 9, 2023

  1. Wrap docstring lines at 72ish characters 

    [PEP 8][0] actually specifies a maximum line length of 72 characters for
"flowing long blocks of text with fewer structural restrictions
(docstrings or comments)". I extended that to 76 characters for indented
docstrings, because the reason for having 72 characters is so that the
eyes don't have to travel horizontally too much. Indentation basically
moves the start of the text block to the right, so for text blocks like
this, the line length limit can move with the indentation.
Some of these lines where even beyond 79 characters, so I had to touch
them anyways to make `flake8` happy and while at it, I also changed them
to be (mostly) PEP 8 compliant.

[0]: https://peps.python.org/pep-0008/#maximum-line-length
    @gnn
    gnn committed Aug 9, 2023
    Configuration menu
    Copy the full SHA
    6c87ee9 View commit details
    Browse the repository at this point in the history

  2. Shorten commented line to be below 79 characters

    @gnn
    gnn committed Aug 9, 2023
    Configuration menu
    Copy the full SHA
    a1423b9 View commit details
    Browse the repository at this point in the history

  3. Shorten line to below 79 characters 

    Moving `OsmBuildingsSynthetic.n_amenities_inside.type` into a variable
is done to work around a bug in `black`. Without this, `black` would
always reformat the line to be exactly 80 characters, even if a limit of
79 is set, making `flake8` complain.
    @gnn
    gnn committed Aug 9, 2023
    Configuration menu
    Copy the full SHA
    3aae1d4 View commit details
    Browse the repository at this point in the history

  4. Reformat with black

    @gnn
    gnn committed Aug 9, 2023
    Configuration menu
    Copy the full SHA
    e0a702b View commit details
    Browse the repository at this point in the history

  5. Add a Tasks type alias to the datasets module 

    This helps when specifying subclasses of `Dataset`. Since there was
already a `Tasks` class inside the module, that class got renamed to
`Tasks_` because it could not be used for the purposes of the type.
    @gnn
    gnn committed Aug 9, 2023
    Configuration menu
    Copy the full SHA
    4562882 View commit details
    Browse the repository at this point in the history

  6. Replace partials of Dataset with subclasses 

    Make these subclasses `@dataclass`es, just like the original `Dataset`,
so that default values for the fields are picked up by the constructor.
That way we don't have to specify a constructor that calls the
superclass constructor, but can rely on the default implementation for
`__init__`.
The `dependencies=[]` line present in nearly all uses of `partial` has
been removed because it was unnecessary. The `dependencies` parameter is
usually specified when the datasets are instantiated and even if not, an
empty list of dependencies is the default anyways. Actually, an empty
tuple is the default, but that doesn't make a difference.
The `tasks=(define_mv_grid_districts)` line has been changed to drop the
parenthesis because the parenthesis where superfluous in this case.
The `tasks=(download_mastr_data,)` line has not been changed, because
even though `Dataset`s should handle a callable and a tuple
containing exactly one callable identically for the tasks attribute, I
didn't want to risk introducing any bugs.
    @gnn
    gnn committed Aug 9, 2023
    Configuration menu
    Copy the full SHA
    ecf2a63 View commit details
    Browse the repository at this point in the history