-
Notifications
You must be signed in to change notification settings - Fork 13
Auto update toml file by adding a script for sphinx docs #315
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
base: main
Are you sure you want to change the base?
Conversation
Newly generated html with this code looks as |
609fd0c
to
354fd2f
Compare
7aadcf1
to
8f6abfd
Compare
from scaler.config.mixins import ConfigType | ||
|
||
|
||
def find_project_root(marker: str = "pyproject.toml") -> pathlib.Path: |
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.
my initial solution was to hardcode this logic but decided to opt for this to make it robust incase directory structure change in future
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.
Do not forget to add the generated config file to the .gitignore
.
try: | ||
module = importlib.import_module(module_name) | ||
for name, obj in inspect.getmembers(module, inspect.isclass): | ||
if dataclasses.is_dataclass(obj) and obj.__module__ == module_name: |
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'd prefer if we checked the section classes based on being a sub-classes of a Config
mixin, instead of relying on dataclasses
.
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.
@rafa-be : To make this work, we need to introduce a marker class (lets name it ConfigSection
) & make all Config classes such as ClusterConfig
, SchedulerConfig
etc to inherit from it. By inheriting from ConfigSection, a class is clearly designated for inclusion in the generated TOML configuration. This "opt-in" approach ensures the script only targets the intended classes without relying on implementation details or location.
However, @sharpener6 is very strict about using inheritance & i dont think this warrants the strict requirements. Please lemme know your thoughts
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 can think of other ways.
Using decorators
def config_section(cls):
cls._is_config_section = True # Add an attribute to the class
return cls
@config_section # Apply the decorator right above the class definition
@dataclasses.dataclass
class ClusterConfig:
......
another idea is to use class atribute:
@dataclasses.dataclass
class ClusterConfig:
_is_config_section = True # Manually add the marker attribute
scheduler_address: ZMQConfig
# ... rest of the class
8f6abfd
to
66d7307
Compare
No description provided.