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

Simplify Objective class args and docstrings #879

Open
f0uriest opened this issue Feb 15, 2024 · 3 comments · May be fixed by #1258
Open

Simplify Objective class args and docstrings #879

f0uriest opened this issue Feb 15, 2024 · 3 comments · May be fixed by #1258
Assignees
Labels
hackathon Stuff to work on during hackathon interface New feature or request to make the code more usable or compatibility with another code low priority Nice to have, but not needed right away objectives Adding or improving objective functions

Comments

@f0uriest
Copy link
Member

We have an ever growing number of objectives, with pretty much all of the init methods being something like this

def __init__(self, thing, 
target, 
bounds, 
weight, 
normalize, 
... (other args generic to all objectives), 
arg1_specific_to_this_objective, 
arg2_specific_to_this_objective
):

And each of the args is documented separately in each class. This means that if we want to add a new property generic to all objectives (like we did with loss_function, normalize, deriv_mode, etc) we have to add it to ALL of the init methods and docstrings

A cleaner method that requires less duplication could be something like

def __init__(self, 
thing, 
arg1_specific_to_this_objective, 
arg2_specific_to_this_objective, 
**kwargs_generic_to_all_objectives
):

Doing this in combination with something like https://github.com/AntoineD/docstring-inheritance would allow us to reduce the amount of redundant docstrings in the code while still having all the args documented to the user in an IDE or docs page. We could also get fancy and override the subclass __signature__ attribute to include the extra args from the parent class.

@f0uriest f0uriest changed the title Simply Objective class args and docstrings Simplily Objective class args and docstrings Feb 15, 2024
@f0uriest f0uriest changed the title Simplily Objective class args and docstrings Simplify Objective class args and docstrings Feb 15, 2024
@f0uriest
Copy link
Member Author

f0uriest commented Mar 7, 2024

Something like this might be easier:

import inspect

class Base():
    """Base docstring"""
    
    def __init__(self, *, base_arg1="auto", base_arg2=None):
        self.base_arg1 = base_arg1
        self.base_arg2 = base_arg2
        
    # https://docs.python.org/3/reference/datamodel.html#object.__init_subclass__
    def __init_subclass__(cls, /, **kwargs):
        super().__init_subclass__(**kwargs)
        cls.__doc__ = cls.__doc__ + Base.__doc__
        sig = inspect.signature(cls)
        sig = sig.replace(parameters=tuple(sig.parameters.values())[:-1] + tuple(inspect.signature(Base).parameters.values()))
        cls.__signature__ = sig
    
    
class Derived(Base):
    """Derived docstring"""
    
    def __init__(self, arg1, arg2, **kwargs):
        self.arg1 = arg1
        self.arg2 = arg2
        super().__init__(**kwargs)

This seems to correctly set the docstring and signature for the derived class, at least in jupyter

@f0uriest f0uriest added the hackathon Stuff to work on during hackathon label Mar 7, 2024
@dpanici
Copy link
Collaborator

dpanici commented Jun 25, 2024

keyword-only args in addition to above, try for just Objectives @f0uriest

@dpanici dpanici added low priority Nice to have, but not needed right away interface New feature or request to make the code more usable or compatibility with another code objectives Adding or improving objective functions labels Aug 20, 2024
@YigitElma YigitElma self-assigned this Sep 21, 2024
@YigitElma
Copy link
Collaborator

Would we like to inherit the docstrings for only documentation website purposes or do we also want to see the documentation when we hover on the class name in VSCode? I would prefer the latter but it is a bit harder I guess.

@YigitElma YigitElma linked a pull request Sep 21, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hackathon Stuff to work on during hackathon interface New feature or request to make the code more usable or compatibility with another code low priority Nice to have, but not needed right away objectives Adding or improving objective functions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants