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

Cleaned up top-level directory, document run_sims #169

Merged
merged 9 commits into from
Sep 28, 2023

Conversation

elaine-ran
Copy link
Contributor

  1. cleaned up the top-level directory
  2. added pyproject.toml in order to ensure notebooks ran smoothly
  3. changed load_vars to load in a dictionary instead of multiple variables
  4. Added descriptions to parts of run_sims (not yet done)
  5. added paper folder for PR between joss-paper branch and dev-massdist
  6. added code to .github/workflows to update paper on push
  7. changed read_yaml, load_vars, make_sz_cluster to accurately work with new system

should resolve issues #168 #167 #163 #110
Issue #114 still in progress

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@samueldmcdermott
Copy link
Contributor

samueldmcdermott commented Sep 25, 2023

@elaine-ran I'll do a first pass on this review

  • it looks like the paper action needs some contents in order to run (it's not sufficient to have a blank paper.md file). There should be sample files on the joss website, or of course feel free to use the files in deepcmbsim. Apologies for having given you bad guidance here!
  • the second fail is from pytest. I think there are multiple errors: some from syntax (unexpected line breaks in the assert command) and some from an import error (saying that there is no simsz package; seems only to be happening in test_utils for some reason). Try correcting the first and we can go from there

@evavagiakis
Copy link
Collaborator

There should be a populated paper.md file in the joss paper branch of the directory, did you find that @elaine-ran? I am currently seeing some conflicting files here:
make_sz_cluster.py
massdist.h5
run_sims.ipynb
tests/test_simtools.py
tests/test_szcluster.py
tests/test_utils.py

@elaine-ran
Copy link
Contributor Author

There should be a populated paper.md file in the joss paper branch of the directory, did you find that @elaine-ran? I am currently seeing some conflicting files here: make_sz_cluster.py massdist.h5 run_sims.ipynb tests/test_simtools.py tests/test_szcluster.py tests/test_utils.py

I copied the paper.md file from the joss-paper branch into this one, and I think that resolved the issue? Since the .github/worksflows/paper.yml check is successful.

@samueldmcdermott
Copy link
Contributor

samueldmcdermott commented Sep 27, 2023

the paper action seems to be fine, but the tests actions still seem to be failing, and there are additional conflicts related to files that you’ve changed @elaine-ran — essentially, github doesn’t know how to reconcile changes you’ve introduced in several files (the ones that @evavagiakis points to). This might be most effectively resolved synchronously — I’m happy to meet tomorrow AM or Friday PM if you want to have a co-coding session

@samueldmcdermott samueldmcdermott merged commit 49f1cbb into dev-massdist Sep 28, 2023
@samueldmcdermott
Copy link
Contributor

there were still errors in tests but we can fix those downstream

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