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

135 remove unused class structure #143

Merged
merged 3 commits into from
Aug 23, 2023

Conversation

kbanker
Copy link
Contributor

@kbanker kbanker commented Aug 18, 2023

This will remove the class structure and update run_sims with that. (1 commit was mistakenly pushed directly to dev_massdist, but should actually be here). The next step will be implementing a cluster dict as input to the main methods that we use.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@evavagiakis
Copy link
Collaborator

@kbanker for some reason the Python Package using Conda / build-linux check has failed. Do you know why that could be?

@kbanker
Copy link
Contributor Author

kbanker commented Aug 21, 2023

@kbanker for some reason the Python Package using Conda / build-linux check has failed. Do you know why that could be?

I think it's because the tests assume the previous class structure and haven't been updated. I'll make an issue to update and expand the tests. They probably need to be changed anyways since we've added lots of functionality since they were written.

@evavagiakis
Copy link
Collaborator

Wanted to check in on this request @kbanker and see if we can resolve it by Friday

@kbanker
Copy link
Contributor Author

kbanker commented Aug 23, 2023

Wanted to check in on this request @kbanker and see if we can resolve it by Friday

@evavagiakis, I think we're good to merge this (and ignore the Python Package using Conda / build-linux failing) until we update the tests. Mainly because its not worth forcing the tests to pass (as they lose all meaning), but actually rewriting them will take a while, probably longer than I have before Friday.

If you prefer, I could just comment out/deactivate the tests for now, and rewrite them properly in the fall?

Copy link
Collaborator

@evavagiakis evavagiakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Class structure removal looks good, unit check is failing but we will rewrite later

@evavagiakis evavagiakis merged commit 0cf1246 into dev-massdist Aug 23, 2023
1 check failed
@kbanker kbanker deleted the 135-remove-unused-class-structure branch August 23, 2023 17:44
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