2
\$\begingroup\$

I am looking for a code review on the following code. Say I have a simple example class, something like the following:

from dataclasses import dataclass
@dataclass
class MyClass:
 attribute1: int
 attribute2: int
 attribute3: int
 attribute4: int

and a config.ini file that contains the following:

[DefaultInstance1]
attribute1=1
attribute2=2
attribute3=3
attribute4=4
[DefaultInstance2]
attribute1=1
attribute2=4
attribute3=9
attribute4=16

I want to be able to simply add Defaults to my config and make them directly available to the user. After a lot of thinking I came up with the following pattern:

import configparser
from types import SimpleNamespace
def create_instances_from_config(config_file):
 config = configparser.ConfigParser()
 config.read(config_file)
 
 instances = {}
 for section in config.sections():
 if section.startswith('DefaultInstance'):
 options = config[section]
 instance = CustomClass(
 options.get('attribute1'),
 options.get('attribute2'),
 options.get('attribute3'),
 options.get('attribute4')
 )
 instances[section] = instance
 
 return SimpleNamespace(**instances)
preconfigured = create_instances_from_config(config)

Now I can import 'preconfigured' in any other file, and pass any preconfigured CustomClass instance around with easy namespacing, something like: preconfigured.DefaultInstance1. It looks pretty clean to me, however, I was wondering if there were any other thoughts about this approach. Are there other, maybe more simple, or less error prone ways to do this? Am I overcomplicating things? Also, I would love to read more about what I am trying to do here, are there specific names for these type of operations? Any feedback or thoughts would be very much appreciated! Thanks! :)

asked Nov 22, 2023 at 11:11
\$\endgroup\$

2 Answers 2

1
\$\begingroup\$

I like it! It looks good.

finding sections

 for section in config.sections():
 if section.startswith('DefaultInstance'):

If you need to do such prefix matching, then fine, this works great, with linear cost to scan the whole .ini file.

For the sequentially numbered sections in your example, it would make more sense to probe them directly. Iterate over positive integers, and ask whether section[f"DefaultInstance{i}"] exists, perhaps using .get().

informative identifier

create_instances_from_config does not exactly seem like a terrific name for your Public API. You have not revealed the use case, the business motivation for this code. Hopefully it offers some vocabulary words that are more informative than "instance", which seems rather dry and synthetic.

Similarly, I'm not crazy about preconfigured.
defaults, maybe?

parameters

Your example code might have passed in "DefaultInstance" as an input argument -- no biggie.

Similarly for attributes 1 .. 4. But this is sort of an XY problem. Your actual code of interest might need such hardcodes.

Recall that glom can help with accessing nested dicts, if that's the underlying pain point you were trying to address.

answered Nov 23, 2023 at 2:23
\$\endgroup\$
1
\$\begingroup\$

Is Something Missing?

It seems to me something is missing from your code. You say in part:

Now I can import 'preconfigured' in any other file, and pass any preconfigured CustomClass instance around ...

I am focusing on the phrase "and pass any preconfigured CustomClass instance around" with emphasis on the word any. Yet your code instantiates an instance of of a specific class MyClass, which is not defined. In your introduction to your solution you presented class MyClass, which I assume would be an example of such an any class that you might want to instantiate. If this is so, then your function create_instances_from_config should take as an additional argument the class you wish to instantiate. For example,

import configparser
from types import SimpleNamespace
def create_instances_from_config(config_file, cls):
 config = configparser.ConfigParser()
 config.read(config_file)
 instances = {}
 for section in config.sections():
 if section.startswith('DefaultInstance'):
 options = config[section]
 instance = cls(
 int(options.get('attribute1')),
 int(options.get('attribute2')),
 int(options.get('attribute3')),
 int(options.get('attribute4'))
 )
 instances[section] = instance
 return SimpleNamespace(**instances)
if __name__ == '__main__':
 from dataclasses import dataclass
 @dataclass
 class MyClass:
 attribute1: int
 attribute2: int
 attribute3: int
 attribute4: int
 preconfigured = create_instances_from_config('test.ini', MyClass)
 print(preconfigured.DefaultInstance1)

Prints:

MyClass(attribute1=1, attribute2=2, attribute3=3, attribute4=4)

Or have I missed something in your code? Also note that your class MyClass defines its attributes as being of type int, but option values from a config file are strings and therefore they must be converted before instantitaing MyClass.

Additional Suggestions

It's a small matter, but your argument name to create_instances_from_config is named config_file. I tend to think of a file as what is returned from the built-in open function. Perhaps a better name for this argument might be config_path. In any event, using typing or a docstring would make this clearer:

def create_instances_from_config(config_path: str, cls: type) -> SimpleNamespace:
 ...

Adding an additional docstring describing what the function does would be a plus.

A More Generic Version

This may not be at all relevant to your situation, but if you needed a more generic version that did not assume anything about the actual option names and their value types, then you could return a dictionary of dictionaries. I would male create_dict_from_config a method of class DefaultConfig that would allow you to create subclasses that override this method to convert the option values to the required types. For example:

import configparser
class DefaultConfig:
 def create_dict_from_config(self, config_path: str) -> dict:
 """Reads in a configuration creating a dictionary whose
 keys are the section names that begin with 'DefaultInstance'
 and whose values are dictionaries of option-name: option-value pairs.
 For example, if the configuration file is ...
 [DefaultInstance1]
 attribute1=1
 attribute2=2
 attribute3=3
 attribute4=4
 [DefaultInstance2]
 attribute1=1
 attribute2=4
 attribute3=9
 attribute4=16
 [Some Other Sections]
 ...
 ... then this function returns the following dictionary:
 {
 'DefaultInstance1':
 {'attribute1': '1', 'attribute2': '2', 'attribute2': '3', 'attribute2': '4'},
 'DefaultInstance2':
 {'attribute1': '1', 'attribute2': '4', 'attribute2': '9', 'attribute2': '16'},
 }
 """
 config = configparser.ConfigParser()
 config.read(config_path)
 sections = {}
 for section in config.sections():
 if section.startswith('DefaultInstance'):
 sections[section] = dict(config[section])
 return sections
if __name__ == '__main__':
 from dataclasses import dataclass
 @dataclass
 class MyClass:
 attribute1: int
 attribute2: int
 attribute3: int
 attribute4: int
 class MyDefaultConfig(DefaultConfig):
 def create_dict_from_config(self, config_path: str) -> dict:
 """Coverts the option values to int types."""
 sections = DefaultConfig.create_dict_from_config(self, config_path)
 for section_name, section_dict in sections.items():
 sections[section_name] = {k: int(v) for k, v in section_dict.items()}
 return sections
 my_default_config = MyDefaultConfig()
 preconfigured = my_default_config.create_dict_from_config('test.ini')
 default_config_1 = MyClass(**preconfigured['DefaultInstance1'])
 print(default_config_1)

Prints:

MyClass(attribute1=1, attribute2=2, attribute3=3, attribute4=4)
answered Nov 25, 2023 at 13:30
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.