-
Notifications
You must be signed in to change notification settings - Fork 4
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.
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
Documentation/rli datasets #1138
Conversation
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.
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.
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.
…ace-partial-with-class Replace `partial` applications of `Dataset` with `Dataset` subclasses
Namely `egon.data.datasets.Dependencies` and `egon.data.datasets.Tasks`. Hopefully this makes the documentation of dataset subclasses using `@dataclass` less confusing. Solution taken from a helpful [answer] on stackoverflow. [0]: https://stackoverflow.com/a/67483317
At least all those, which Sphinx highlights in boldface.
IMHO it's more readable this way. Feel free to remove the commit if you don't concur.
By removing an unnecessary level of indentation.
Courtesy of `black`.
Instead of relying on `@dataclass` to provide one. This makes the constructor's signature less confusing. I also allows not specifying types for the class variables, because `@dataclass` no longer needs to pick the up automatically, as we are manually specifying them in the superclass constructor call. This means we have less stuff to import, most notably we no longer need to import `Tasks`. This also allows us to work around the broken display of `mv_grid_districts_setup`'s `tasks`. That was only a single function and for some reason Sphinx did not display that one correctly. Probably because it tried to use `str` instead of `repr` to render the function, because boxing the function in a one-tuple would fix the display. Anyway. Not putting the task on a class attribute but specifying it directly in the constructor means that no class attribute is displayed in the documentation, thus sidestepping the issue.
…sets/alternative-partial-replacements
…ted with partial Propose alternative replacements for partial.
src/egon/data/datasets/emobility/heavy_duty_transport/h2_demand_distribution.py
…set documentation
|
||
* explain scenario parameters | ||
|
||
* run params (all in meta file?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This question mark looks a bit strange...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from the small comment related to the question mark, this branch looks good to me.
In this branch the following datasets are documented:
As some datasets were created using partial and documenting them was not possible, their setup was changed as well with PR #1146.