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

Sort out if machine is a needed argument to tool base class init #11

Open
brendenpelkie opened this issue Oct 10, 2023 · 2 comments
Open
Labels
discussion Community input is requested

Comments

@brendenpelkie
Copy link
Collaborator

brendenpelkie commented Oct 10, 2023

Tool base class takes a machine but this doesn't seem to be needed. Is it? I took it out while debugging and never looked into if that will break something or not. I think there was some issue on instantiation of a tool if machine attribute was not defined in base class instantiation.

@bsubbaraman
Copy link
Member

bsubbaraman commented Oct 11, 2023

It shouldn't be needed; instead, the _machine attribute is set in load_tool(). Anecdotally I've been using the repo for a while without passing the machine explicitly and hadn't run into any errors.

I did notice that the Pipette module takes a machine instance as a parameter, which I think is redundant/not necessary.

@bsubbaraman bsubbaraman added the discussion Community input is requested label Oct 11, 2023
@MariaPoliti
Copy link
Collaborator

I've removed the redundant attribute _machine from the Pipette and WebCamera tools. I've kept it in the base Tool class, but I am happy to remove it if it is considered uneccessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Community input is requested
Projects
None yet
Development

No branches or pull requests

3 participants