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

add badelf and warren_lab apps 🎅 #547

Merged
merged 67 commits into from
Dec 22, 2023
Merged

add badelf and warren_lab apps 🎅 #547

merged 67 commits into from
Dec 22, 2023

Conversation

SWeav02
Copy link
Contributor

@SWeav02 SWeav02 commented Dec 19, 2023

Add a badelf app with a class oriented version of the badelf algorithm. Also add a warrenapp app which contains many convenience workflows for the warren lab.

@SWeav02
Copy link
Contributor Author

SWeav02 commented Dec 20, 2023

@jacksund I think I've made adjustments based off of each of your comments. I'm working on running the test suite now (#546 (comment)) and prepping for a new release.

@jacksund
Copy link
Owner

thanks for going through! Normally release prep should be its own PR -- also I'm the only one that has permissions to do some of the steps listed in that guide.

I'm going to go through and make some edits on your code, then merge. I may move the release changes into a separate PR too

@jacksund jacksund marked this pull request as ready for review December 20, 2023 23:39
@jacksund
Copy link
Owner

fyi -- the prebuilt database will be broken for this PR because you deleted the migration files in one of your recent commits 0dd5ce0. These migrations need to be re-added and then you can use simmate database update with the prebuilt db

I learned earlier this year that we were incorrectly deleting/restarting migrations every time we had an update. We actually want to commit migrations to the repo! That way users can easily update their database between simmate versions / new releases. Other django apps do this too (here is allauth's migrations as an example here)

@SWeav02
Copy link
Contributor Author

SWeav02 commented Dec 21, 2023

fyi -- the prebuilt database will be broken for this PR because you deleted the migration files in one of your recent commits 0dd5ce0. These migrations need to be re-added and then you can use simmate database update with the prebuilt db

I learned earlier this year that we were incorrectly deleting/restarting migrations every time we had an update. We actually want to commit migrations to the repo! That way users can easily update their database between simmate versions / new releases. Other django apps do this too (here is allauth's migrations as an example here)

Oops good to know. I'll add them back and commit them.

@jacksund
Copy link
Owner

also not sure if you saw this: https://github.com/SWeav02/simmate/pull/20. Once merged, it will update this PR too

@SWeav02
Copy link
Contributor Author

SWeav02 commented Dec 21, 2023

also not sure if you saw this: SWeav02#20. Once merged, it will update this PR too

I'm looking through it now 😆

@jacksund jacksund changed the title Add badelf and warrenapp apps add badelf and warren_lab apps 🎅 Dec 22, 2023
@jacksund
Copy link
Owner

@SWeav02 take a look through & merge https://github.com/SWeav02/simmate/pull/22 when you get the chance. I removed badelf and warren_lab from the default apps for now and made some minor changes elsewhere. This will put everything in a state where I can actually merge your PR. Then the edits needed in order to allow these apps as defaults will be handled in separate PRs

@jacksund
Copy link
Owner

only the linux ci is failing but its unrelated to this PR. The error is because a new pydantic version was released 6 hrs ago https://anaconda.org/conda-forge/pydantic.

merging and will address the pydantic issue elsewhere

@jacksund jacksund merged commit f6256ae into jacksund:main Dec 22, 2023
5 of 7 checks passed
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.

2 participants