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

added kwargs to alternate constructor classmethods #318

Merged
merged 1 commit into from
Apr 19, 2022

Conversation

tylerflex
Copy link
Collaborator

These get passed to the init functions.

Example

class Box(Geometry):

    @classmethod
    def from_bounds(cls, rmin: Coordinate, rmax: Coordinate, **kwargs):

        def get_center(pt_min: float, pt_max: float) -> float:
            """Returns center point based on bounds along dimension."""
            if np.isneginf(pt_min) and np.isposinf(pt_max):
                return 0.0
            return (pt_min + pt_max) / 2.0

        center = tuple(get_center(pt_min, pt_max) for pt_min, pt_max in zip(rmin, rmax))
        size = tuple((pt_max - pt_min) for pt_min, pt_max in zip(rmin, rmax))
        return cls(center=center, size=size, **kwargs)

Doing it this way means you can call

sim = Simulation.from_bounds(rmin=(0,0,0), rmax=(1,1,1), grid_size=(.1, .1, .1), run_time=1e-12)

mon = FluxMonitor.from_bounds(rmin=(0,0,0), rmax=(0,1,1), freqs=[1e14])

automatically.

@tylerflex tylerflex requested a review from momchil-flex April 19, 2022 00:49
@tylerflex tylerflex linked an issue Apr 19, 2022 that may be closed by this pull request
@momchil-flex
Copy link
Collaborator

Pretty neat. My only comment is that we're getting a bunch of "black" and "lint" commits in the commit tree. It's not the worst thing in the world but would be nice to squash them before merging to develop.

You can do this manually with the git rebase -i HEAD~2 worfklow (a bit annoying, I don't know if there's a faster way). The github "squash and merge" should only be used if there has been nothing else merged to develop meanwhile, so while it will work for this branch right now, we shouldn't generally use it.

@tylerflex
Copy link
Collaborator Author

yea good point about the black commits. I'll be more mindful of that moving forward.

@twhughes twhughes force-pushed the tyler/classmethod_kwargs branch from e350866 to c7a35df Compare April 19, 2022 22:24
@tylerflex tylerflex merged commit e65e02c into develop Apr 19, 2022
@tylerflex tylerflex deleted the tyler/classmethod_kwargs branch April 19, 2022 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kwargs in class methods
2 participants