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

Prep merge #349

Merged
merged 3 commits into from
Jan 5, 2024
Merged

Prep merge #349

merged 3 commits into from
Jan 5, 2024

Conversation

adamltyson
Copy link
Member

Combines two very similar functions and also removes a seemingly redundant function arguement.

@sfmig I've somewhat naively updated the benchmarking code, but I may have done this wrong.

Closes #272

@adamltyson
Copy link
Member Author

brainmapper updated in brainglobe/brainglobe-workflows#66

@willGraham01
Copy link
Collaborator

willGraham01 commented Jan 4, 2024

Worth flagging here that when this update and the matching one on workflows goes in, we should make new releases for both. (In particular, the workflows PR should force a minimum version of cellfinder that corresponds to the new release).

And another thing I've just noticed:

  • Publishing a new release requires the brainmapper CI check to pass, but it won't without the latest update to workflows.
  • Publishing a new workflows release requires CI to pass, which requires the new version of cellfinder

So theoretically we can never publish this release (need to release cellfinder to release workflows, and the converse is also true).

In the interest of resolving this, I guess we make the brainmapper CI check in this repo optional when creating a release and/or merging a PR? That way we at least have a check which'll indicate when things can potentially go wrong and need to be fixed in workflows too.

@adamltyson
Copy link
Member Author

In the interest of resolving this, I guess we make the brainmapper CI check in this repo optional when creating a release and/or merging a PR? That way we at least have a check which'll indicate when things can potentially go wrong and need to be fixed in workflows too.

Yeah I think that's the way.

@willGraham01
Copy link
Collaborator

Yeah I think that's the way.

Have added this change to this branch.

@willGraham01
Copy link
Collaborator

@adamltyson do you want me to merge this, publish the release, and then try re-running the CI on brainglobe/brainglobe-workflows#66 ?

@adamltyson
Copy link
Member Author

Sounds good

@willGraham01 willGraham01 merged commit c38399a into main Jan 5, 2024
13 of 14 checks passed
@willGraham01 willGraham01 deleted the prep-merge branch January 5, 2024 09:28
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.

How are prep_classification and prep_training different?
2 participants