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.

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

Cities init method arguments #143

Closed
gonzaponte opened this issue Feb 25, 2017 · 4 comments · Fixed by #259
Closed

Cities init method arguments #143

gonzaponte opened this issue Feb 25, 2017 · 4 comments · Fixed by #259
Assignees
Labels

Comments

@gonzaponte
Copy link
Collaborator

We need to discuss whether it is more convenient for the cities constructors to list just their own arguments and take the ones belonging to the classes they inherit from as keywords or to list them all and make it more explicit.

@jacg
Copy link
Collaborator

jacg commented Mar 6, 2017

Please create a branch with the proposed change and link to it here so that we can compare and contrast the two approaches.

@gonzaponte
Copy link
Collaborator Author

In gonzaponte/citiesargs you can find a draft of the code for this issue (it is a couple of commits behind nextic/master, but I will rebase). I have made two things:

  • Remove from base classes (SensorResponseCity, CalibratedCity, etc.) all arguments belonging to the classes they inherit from, keeping only those owned by the class.
  • Remove from implementation classes (Diomira, Irene, etc.) all arguments belonging to the base classes. This means that some explicit constructors which only job was to call the superclass constructor are not needed, so they have been also removed. In the remaining ones, the call is reduced to a single line.

I have done this in separate commits so it can be easily reverted.

@jacg
Copy link
Collaborator

jacg commented May 9, 2017

@jjgomezcadenas You and I should look at this with peras and make a decision.

@jjgomezcadenas
Copy link
Collaborator

Agreed, next week!

@jacg jacg closed this as completed in #259 Jun 28, 2017
jacg added a commit that referenced this issue Jun 28, 2017
1. The driver functions for each city (the city name in uppercase)
   have been removed. They are all replaced with a single drive
   classmethod in City. This simplifies the code considerably and
   removes the copy-n-paste mess that has been growing an each new
   city was added.

2. The city constructors no longer take and pass arguments explicitly
   by name, this is now achieved with **kwds. This closes #143 and has
   a number of consequences Significant noise reduction in constructor
   definitions.  The config files become the sole source of

   + values of the parameters
   + documentation of what parameters are needed by each kind of city

3. The config files are now written in Python syntax and parsed with
   Python's built-in parser. Our custom config parser for our ad-hoc
   config syntax has been thrown away and replaced with Python's
   parser. Do not re-invent the wheel.

4. The ability to inculde config files within other config files has
   been added, allowing the use of hierarchical configurations. Thus
   the default configuration of some base city does not need to be
   repeated in each concrete city's config file, as in can be
   included.

5. A new configuration printer has been added with the key abilities
   to

   + report the config file which provides any given value in an
     active configuration

   + highlight cases where a value set in one config file is
     overridden by a setting in another one.

    Improvements to the config printer are envisaged:

    + Displaying the comments from the config files.

    + Displaying the units used in the config files.

6. The mandatory -c (config file) command line option has been turned
   into a positional argument.

(#203 has not been addressed at all here.)
jacg added a commit that referenced this issue Sep 9, 2017
1. The driver functions for each city (the city name in uppercase)
   have been removed. They are all replaced with a single drive
   classmethod in City. This simplifies the code considerably and
   removes the copy-n-paste mess that has been growing an each new
   city was added.

2. The city constructors no longer take and pass arguments explicitly
   by name, this is now achieved with **kwds. This closes #143 and has
   a number of consequences Significant noise reduction in constructor
   definitions.  The config files become the sole source of

   + values of the parameters
   + documentation of what parameters are needed by each kind of city

3. The config files are now written in Python syntax and parsed with
   Python's built-in parser. Our custom config parser for our ad-hoc
   config syntax has been thrown away and replaced with Python's
   parser. Do not re-invent the wheel.

4. The ability to inculde config files within other config files has
   been added, allowing the use of hierarchical configurations. Thus
   the default configuration of some base city does not need to be
   repeated in each concrete city's config file, as in can be
   included.

5. A new configuration printer has been added with the key abilities
   to

   + report the config file which provides any given value in an
     active configuration

   + highlight cases where a value set in one config file is
     overridden by a setting in another one.

    Improvements to the config printer are envisaged:

    + Displaying the comments from the config files.

    + Displaying the units used in the config files.

6. The mandatory -c (config file) command line option has been turned
   into a positional argument.

(#203 has not been addressed at all here.)
jacg added a commit that referenced this issue Sep 9, 2017
1. The driver functions for each city (the city name in uppercase)
   have been removed. They are all replaced with a single drive
   classmethod in City. This simplifies the code considerably and
   removes the copy-n-paste mess that has been growing an each new
   city was added.

2. The city constructors no longer take and pass arguments explicitly
   by name, this is now achieved with **kwds. This closes #143 and has
   a number of consequences Significant noise reduction in constructor
   definitions.  The config files become the sole source of

   + values of the parameters
   + documentation of what parameters are needed by each kind of city

3. The config files are now written in Python syntax and parsed with
   Python's built-in parser. Our custom config parser for our ad-hoc
   config syntax has been thrown away and replaced with Python's
   parser. Do not re-invent the wheel.

4. The ability to inculde config files within other config files has
   been added, allowing the use of hierarchical configurations. Thus
   the default configuration of some base city does not need to be
   repeated in each concrete city's config file, as in can be
   included.

5. A new configuration printer has been added with the key abilities
   to

   + report the config file which provides any given value in an
     active configuration

   + highlight cases where a value set in one config file is
     overridden by a setting in another one.

    Improvements to the config printer are envisaged:

    + Displaying the comments from the config files.

    + Displaying the units used in the config files.

6. The mandatory -c (config file) command line option has been turned
   into a positional argument.

(#203 has not been addressed at all here.)
jacg added a commit that referenced this issue Sep 10, 2017
1. The driver functions for each city (the city name in uppercase)
   have been removed. They are all replaced with a single drive
   classmethod in City. This simplifies the code considerably and
   removes the copy-n-paste mess that has been growing an each new
   city was added.

2. The city constructors no longer take and pass arguments explicitly
   by name, this is now achieved with **kwds. This closes #143 and has
   a number of consequences Significant noise reduction in constructor
   definitions.  The config files become the sole source of

   + values of the parameters
   + documentation of what parameters are needed by each kind of city

3. The config files are now written in Python syntax and parsed with
   Python's built-in parser. Our custom config parser for our ad-hoc
   config syntax has been thrown away and replaced with Python's
   parser. Do not re-invent the wheel.

4. The ability to inculde config files within other config files has
   been added, allowing the use of hierarchical configurations. Thus
   the default configuration of some base city does not need to be
   repeated in each concrete city's config file, as in can be
   included.

5. A new configuration printer has been added with the key abilities
   to

   + report the config file which provides any given value in an
     active configuration

   + highlight cases where a value set in one config file is
     overridden by a setting in another one.

    Improvements to the config printer are envisaged:

    + Displaying the comments from the config files.

    + Displaying the units used in the config files.

6. The mandatory -c (config file) command line option has been turned
   into a positional argument.

(#203 has not been addressed at all here.)
jacg added a commit that referenced this issue Sep 15, 2017
1. The driver functions for each city (the city name in uppercase)
   have been removed. They are all replaced with a single drive
   classmethod in City. This simplifies the code considerably and
   removes the copy-n-paste mess that has been growing an each new
   city was added.

2. The city constructors no longer take and pass arguments explicitly
   by name, this is now achieved with **kwds. This closes #143 and has
   a number of consequences Significant noise reduction in constructor
   definitions.  The config files become the sole source of

   + values of the parameters
   + documentation of what parameters are needed by each kind of city

3. The config files are now written in Python syntax and parsed with
   Python's built-in parser. Our custom config parser for our ad-hoc
   config syntax has been thrown away and replaced with Python's
   parser. Do not re-invent the wheel.

4. The ability to inculde config files within other config files has
   been added, allowing the use of hierarchical configurations. Thus
   the default configuration of some base city does not need to be
   repeated in each concrete city's config file, as in can be
   included.

5. A new configuration printer has been added with the key abilities
   to

   + report the config file which provides any given value in an
     active configuration

   + highlight cases where a value set in one config file is
     overridden by a setting in another one.

    Improvements to the config printer are envisaged:

    + Displaying the comments from the config files.

    + Displaying the units used in the config files.

6. The mandatory -c (config file) command line option has been turned
   into a positional argument.

(#203 has not been addressed at all here.)
jmbenlloch pushed a commit to jmbenlloch/IC-1 that referenced this issue Apr 24, 2018
1. The driver functions for each city (the city name in uppercase)
   have been removed. They are all replaced with a single drive
   classmethod in City. This simplifies the code considerably and
   removes the copy-n-paste mess that has been growing an each new
   city was added.

2. The city constructors no longer take and pass arguments explicitly
   by name, this is now achieved with **kwds. This closes next-exp#143 and has
   a number of consequences Significant noise reduction in constructor
   definitions.  The config files become the sole source of

   + values of the parameters
   + documentation of what parameters are needed by each kind of city

3. The config files are now written in Python syntax and parsed with
   Python's built-in parser. Our custom config parser for our ad-hoc
   config syntax has been thrown away and replaced with Python's
   parser. Do not re-invent the wheel.

4. The ability to inculde config files within other config files has
   been added, allowing the use of hierarchical configurations. Thus
   the default configuration of some base city does not need to be
   repeated in each concrete city's config file, as in can be
   included.

5. A new configuration printer has been added with the key abilities
   to

   + report the config file which provides any given value in an
     active configuration

   + highlight cases where a value set in one config file is
     overridden by a setting in another one.

    Improvements to the config printer are envisaged:

    + Displaying the comments from the config files.

    + Displaying the units used in the config files.

6. The mandatory -c (config file) command line option has been turned
   into a positional argument.

(next-exp#203 has not been addressed at all here.)
jacg added a commit to jacg/IC that referenced this issue Sep 14, 2018
1. The driver functions for each city (the city name in uppercase)
   have been removed. They are all replaced with a single drive
   classmethod in City. This simplifies the code considerably and
   removes the copy-n-paste mess that has been growing an each new
   city was added.

2. The city constructors no longer take and pass arguments explicitly
   by name, this is now achieved with **kwds. This closes next-exp#143 and has
   a number of consequences Significant noise reduction in constructor
   definitions.  The config files become the sole source of

   + values of the parameters
   + documentation of what parameters are needed by each kind of city

3. The config files are now written in Python syntax and parsed with
   Python's built-in parser. Our custom config parser for our ad-hoc
   config syntax has been thrown away and replaced with Python's
   parser. Do not re-invent the wheel.

4. The ability to inculde config files within other config files has
   been added, allowing the use of hierarchical configurations. Thus
   the default configuration of some base city does not need to be
   repeated in each concrete city's config file, as in can be
   included.

5. A new configuration printer has been added with the key abilities
   to

   + report the config file which provides any given value in an
     active configuration

   + highlight cases where a value set in one config file is
     overridden by a setting in another one.

    Improvements to the config printer are envisaged:

    + Displaying the comments from the config files.

    + Displaying the units used in the config files.

6. The mandatory -c (config file) command line option has been turned
   into a positional argument.

(next-exp#203 has not been addressed at all here.)
jmbenlloch pushed a commit to jmbenlloch/IC-1 that referenced this issue Oct 16, 2018
1. The driver functions for each city (the city name in uppercase)
   have been removed. They are all replaced with a single drive
   classmethod in City. This simplifies the code considerably and
   removes the copy-n-paste mess that has been growing an each new
   city was added.

2. The city constructors no longer take and pass arguments explicitly
   by name, this is now achieved with **kwds. This closes next-exp#143 and has
   a number of consequences Significant noise reduction in constructor
   definitions.  The config files become the sole source of

   + values of the parameters
   + documentation of what parameters are needed by each kind of city

3. The config files are now written in Python syntax and parsed with
   Python's built-in parser. Our custom config parser for our ad-hoc
   config syntax has been thrown away and replaced with Python's
   parser. Do not re-invent the wheel.

4. The ability to inculde config files within other config files has
   been added, allowing the use of hierarchical configurations. Thus
   the default configuration of some base city does not need to be
   repeated in each concrete city's config file, as in can be
   included.

5. A new configuration printer has been added with the key abilities
   to

   + report the config file which provides any given value in an
     active configuration

   + highlight cases where a value set in one config file is
     overridden by a setting in another one.

    Improvements to the config printer are envisaged:

    + Displaying the comments from the config files.

    + Displaying the units used in the config files.

6. The mandatory -c (config file) command line option has been turned
   into a positional argument.

(next-exp#203 has not been addressed at all here.)
jmbenlloch pushed a commit to jmbenlloch/IC-1 that referenced this issue Oct 16, 2018
1. The driver functions for each city (the city name in uppercase)
   have been removed. They are all replaced with a single drive
   classmethod in City. This simplifies the code considerably and
   removes the copy-n-paste mess that has been growing an each new
   city was added.

2. The city constructors no longer take and pass arguments explicitly
   by name, this is now achieved with **kwds. This closes next-exp#143 and has
   a number of consequences Significant noise reduction in constructor
   definitions.  The config files become the sole source of

   + values of the parameters
   + documentation of what parameters are needed by each kind of city

3. The config files are now written in Python syntax and parsed with
   Python's built-in parser. Our custom config parser for our ad-hoc
   config syntax has been thrown away and replaced with Python's
   parser. Do not re-invent the wheel.

4. The ability to inculde config files within other config files has
   been added, allowing the use of hierarchical configurations. Thus
   the default configuration of some base city does not need to be
   repeated in each concrete city's config file, as in can be
   included.

5. A new configuration printer has been added with the key abilities
   to

   + report the config file which provides any given value in an
     active configuration

   + highlight cases where a value set in one config file is
     overridden by a setting in another one.

    Improvements to the config printer are envisaged:

    + Displaying the comments from the config files.

    + Displaying the units used in the config files.

6. The mandatory -c (config file) command line option has been turned
   into a positional argument.

(next-exp#203 has not been addressed at all here.)
jmbenlloch pushed a commit to jmbenlloch/IC-1 that referenced this issue Oct 16, 2018
1. The driver functions for each city (the city name in uppercase)
   have been removed. They are all replaced with a single drive
   classmethod in City. This simplifies the code considerably and
   removes the copy-n-paste mess that has been growing an each new
   city was added.

2. The city constructors no longer take and pass arguments explicitly
   by name, this is now achieved with **kwds. This closes next-exp#143 and has
   a number of consequences Significant noise reduction in constructor
   definitions.  The config files become the sole source of

   + values of the parameters
   + documentation of what parameters are needed by each kind of city

3. The config files are now written in Python syntax and parsed with
   Python's built-in parser. Our custom config parser for our ad-hoc
   config syntax has been thrown away and replaced with Python's
   parser. Do not re-invent the wheel.

4. The ability to inculde config files within other config files has
   been added, allowing the use of hierarchical configurations. Thus
   the default configuration of some base city does not need to be
   repeated in each concrete city's config file, as in can be
   included.

5. A new configuration printer has been added with the key abilities
   to

   + report the config file which provides any given value in an
     active configuration

   + highlight cases where a value set in one config file is
     overridden by a setting in another one.

    Improvements to the config printer are envisaged:

    + Displaying the comments from the config files.

    + Displaying the units used in the config files.

6. The mandatory -c (config file) command line option has been turned
   into a positional argument.

(next-exp#203 has not been addressed at all here.)
jmbenlloch pushed a commit that referenced this issue Oct 16, 2018
1. The driver functions for each city (the city name in uppercase)
   have been removed. They are all replaced with a single drive
   classmethod in City. This simplifies the code considerably and
   removes the copy-n-paste mess that has been growing an each new
   city was added.

2. The city constructors no longer take and pass arguments explicitly
   by name, this is now achieved with **kwds. This closes #143 and has
   a number of consequences Significant noise reduction in constructor
   definitions.  The config files become the sole source of

   + values of the parameters
   + documentation of what parameters are needed by each kind of city

3. The config files are now written in Python syntax and parsed with
   Python's built-in parser. Our custom config parser for our ad-hoc
   config syntax has been thrown away and replaced with Python's
   parser. Do not re-invent the wheel.

4. The ability to inculde config files within other config files has
   been added, allowing the use of hierarchical configurations. Thus
   the default configuration of some base city does not need to be
   repeated in each concrete city's config file, as in can be
   included.

5. A new configuration printer has been added with the key abilities
   to

   + report the config file which provides any given value in an
     active configuration

   + highlight cases where a value set in one config file is
     overridden by a setting in another one.

    Improvements to the config printer are envisaged:

    + Displaying the comments from the config files.

    + Displaying the units used in the config files.

6. The mandatory -c (config file) command line option has been turned
   into a positional argument.

(#203 has not been addressed at all here.)
jmbenlloch pushed a commit to jmbenlloch/IC-1 that referenced this issue Oct 16, 2018
1. The driver functions for each city (the city name in uppercase)
   have been removed. They are all replaced with a single drive
   classmethod in City. This simplifies the code considerably and
   removes the copy-n-paste mess that has been growing an each new
   city was added.

2. The city constructors no longer take and pass arguments explicitly
   by name, this is now achieved with **kwds. This closes next-exp#143 and has
   a number of consequences Significant noise reduction in constructor
   definitions.  The config files become the sole source of

   + values of the parameters
   + documentation of what parameters are needed by each kind of city

3. The config files are now written in Python syntax and parsed with
   Python's built-in parser. Our custom config parser for our ad-hoc
   config syntax has been thrown away and replaced with Python's
   parser. Do not re-invent the wheel.

4. The ability to inculde config files within other config files has
   been added, allowing the use of hierarchical configurations. Thus
   the default configuration of some base city does not need to be
   repeated in each concrete city's config file, as in can be
   included.

5. A new configuration printer has been added with the key abilities
   to

   + report the config file which provides any given value in an
     active configuration

   + highlight cases where a value set in one config file is
     overridden by a setting in another one.

    Improvements to the config printer are envisaged:

    + Displaying the comments from the config files.

    + Displaying the units used in the config files.

6. The mandatory -c (config file) command line option has been turned
   into a positional argument.

(next-exp#203 has not been addressed at all here.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants