-
Notifications
You must be signed in to change notification settings - Fork 88
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
LINT: few remaining refactor and warning flags to be resolved #191
Comments
Thank you for this cleanup! Most of these issues have accumulated over the past few years, during which testing and linting were apparently not used in development even locally. An exception is Regarding your questions:
Please also make sure to update the tests and tutorials accordingly. |
Thank you for the advice and for updating the I'll take care of those remaining fixes as soon as possible, hopefully later this week. |
Working on getting the Travis build pass again, I resolved many minor
flake8
andpylint
flags in b9a31aa and 42734ff.flake8
is passing now, but there are somepylint
flags remaining that I wasn't yet able to resolve on my own. @ntfrgl, could you give me some advice on those? I know there's other open issues right now, but I think getting the Travis build to pass again would be very useful for all subsequent work and it does not seem to require much more.In order of ascending difficulty (to me):
too many arguments in method
in
climate.EventSeriesClimateNetwork.__init__()
pylint says:Any downsides of configuring pylint to allow for 16 arguments instead of 12?
try: return; except Error:
-clausesin e.g.
core.Grid.Load()
pylint says:Put the return statement after the
try-except
instead?(Or don't re-raise in place at all, as is advised in this SO-thread? Would then lose the additional info though ..)
arguments reordered in overriding methods
in several places (see below), pylint says e.g:
In some cases, arguments are actually just renamed, which is resolvable.
In others, this is due to additional required arguments in the overriding method in question, which have to stand before the optional arguments. This causes a reordering of arguments. But apparently, you can have parameters in the child class which don't feature in the parent class, so long as they are optional!
This concerns
timeseries.JointRecurrenceNetwork.set_fixed_recurrence_rate()
(overriding the parent class methodtimeseries.JointRecurrencePlot.set_fixed_recurrence_rate()
)density
instead ofrecurrence_rate
core.SpatialNetwork.save()
(overriding the parent class methodcore.Network.save()
)filename_network
instead offilename
core.GeoGrid.RegularGrid()
(overriding the parent class methodcore.Grid.RegularGrid()
)lat_grid, lon_grid
instead ofspace_grid
core.SpatialNetwork.Load()
(overriding the parent class methodcore.Network.Load()
)filename_network, filename_grid
instead offilename
climate.ClimateNetwork.Load()
(overriding the parent class methodcore.GeoNetwork.Load()
)filename_similarity_measure
added before optional argsclimate.ClimateData.Load()
(overriding the parent class methodcore.Data.Load()
)time_cycle, time_name="time", latitude_name="lat", longitude_name="lon", data_source=None, file_type="NetCDF"
instead offile_type, dimension_names=None
The first two should be easy fixes (rename), maybe also the following two (put two arguments into one). The last two, however, especially the very last one, seem not so easy to resolve. Any idea of a fix that does not require major restructuring? Or ignore them?
Thanks in advance!
See full pylint report here: https://gist.github.com/fkuehlein/fbc67b94a9425544857a612a3b15d0d3
It contains some further 'convention' flags, but I'll treat those separately.
The text was updated successfully, but these errors were encountered: