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

[Feature Request] Sanitize the OpenSurgery situation #230

Open
gwwatkin opened this issue Jan 29, 2022 · 5 comments
Open

[Feature Request] Sanitize the OpenSurgery situation #230

gwwatkin opened this issue Jan 29, 2022 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@gwwatkin
Copy link
Member

Issue Description

#229 introduces OpenSurgery as a submodule. This might not be desirable because opensurgery doesn't appear to have been actively maintained in the last 2 years and we are using only very little functionality from it. @alexnguyenn feels strongly about it.

Proposed Solutions

  • Contribute to OpenSurgery and Add it to PyPi so it can properly be imported (potentially a lot of work)
  • Copy over the functionality we need. Might run into attribution/license problems and might still have a lot of work to disentangle components and some chance for mistakes
  • Re implement the functionality. A lot of work and a lot of chances for mistakes.

Additional References

https://github.com/alexandrupaler/opensurgery

@gwwatkin gwwatkin added the enhancement New feature or request label Jan 29, 2022
@Keelando
Copy link
Member

Keelando commented Feb 2, 2022

Currently fighting this one in #220

How should I utilize Opensurgery?

I have used submodules before but they are hardly "plug and play". When you clone the repo they are usually empty, and stuff breaks. At the very least we need to update the README to spell out exactly how a user can populate the submodule

Can we just periodically copy the entire folder over to our project? If we are using the repo as a submodule, we still need attribution etc.

@Keelando
Copy link
Member

Keelando commented Feb 3, 2022

Edit: just ran git submodule init, git submodule update.

Also, with git submodules, we can point our main repo to a specific commit of the submodule, correct? This would ensure that when someone runs "git submodule update" it updates to the desired commit/branch of Opensurgery. However, even though the LSC repo points to a certain commit on Opensurgery, the Opensurgery directory will still be empty when someone clones LSC

@gwwatkin
Copy link
Member Author

gwwatkin commented Feb 3, 2022

@Keelando will update the readme. I think it's fine for now to have Opensurgery point to the latest commit. You can clone with git clone --recursive.

@Keelando
Copy link
Member

Keelando commented Feb 3, 2022

I have to admit I'm always a little fuzzy on submodules. As long as the behaviour is reproducible it is good. Just like for dependencies, it is often good to lock the version to avoid incorporating breaking changes. Seems like there haven't been updates to the repo in two years though.

@gwwatkin
Copy link
Member Author

gwwatkin commented Mar 2, 2022

All we are using from OpenSurgery now is the Qentiana class. I think the way to proceed is taking out Qentiana from OpenSurgery and putting into it's own repo and calling it from there. Alternatively we can just turn it into our own resource estimation class and keep it in our repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants