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

Create functions for mask creation that return a ndarray #47

Merged
merged 7 commits into from
Sep 16, 2024

Conversation

jojoelfe
Copy link
Contributor

Hi,

I was wondering if it would be possible to slightly refactor the mask creation so that the underlying functions return the numpy array instead of writing to an mrcfile. This makes it easier to use ttmask as a library.

Here is my attempt to do it (which I thought could be done easily by Copilot but that turned out to be a huge mistake). I've added some basic tests too, but I do think they could be neater.

Feel free to dismiss if you do not like it. Also happy to make any changes you suggest.

@alisterburt
Copy link
Member

Love this! I'm all for it - @milesagraham what do you think?

@milesagraham
Copy link
Member

Hi, this sounds good to me too and makes sense when looking for integration with other teamtomo bits. I've taken a brief look but need to spend time on Friday dedicated to understanding your code (want to keep an understanding of what each line in the ttmask code is doing, for my own benefit as a rookie). Thank you very much for contributing and improving it!

@alisterburt do you want to squash/merge this now?

@alisterburt
Copy link
Member

@milesagraham go ahead! 🙂

@milesagraham milesagraham merged commit a88d3bb into teamtomo:main Sep 16, 2024
7 checks passed
@milesagraham
Copy link
Member

milesagraham commented Sep 16, 2024

Hi, I've gone through all of the changes and understand what you are doing, thanks! Makes so much sense to do this - have merged now

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.

3 participants