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

Fixed Notebook Tests #52

Merged
merged 17 commits into from
Jun 8, 2023
Merged

Fixed Notebook Tests #52

merged 17 commits into from
Jun 8, 2023

Conversation

pjflux2001
Copy link
Member

@pjflux2001 pjflux2001 commented Jun 4, 2023

  • Added a flag file to differentiate between workflow tests
  • Made changes in notebook dir path and few minor changes in notebook itself
  • Added few requirements which were required for staging notebook
  • Upgraded macOS versions as previous ones are deprecated read issue here

Closes #41

@pjflux2001 pjflux2001 changed the title Fixing Notebook Tests Fixed Notebook Tests Jun 4, 2023
Copy link
Contributor

@PatriceJada PatriceJada left a comment

Choose a reason for hiding this comment

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

@pjflux2001 thank you for getting this working. Looks good but I just put up a few concerns. Do we need the production folder? PolyPhy/experiments/jupyter /production/ The path looks very long. Could you also squash your commits to one issue. The conventional way should be one commit per issue fixed, this avoids us getting passed the 2GB that git provides.

@@ -30,6 +30,9 @@ jobs:
python -m pip install flake8 pytest
if [ -f requirements.txt ]; then pip install -r requirements.txt; fi
pip install . -U
- name: Create Flag File
run: |
touch /tmp/flag
Copy link
Contributor

Choose a reason for hiding this comment

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

I have not used this but do we really need it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this is needed. Might not be most efficient way to do it - but I couldn't think of a better approach yet

src/polyphy/profile.json Outdated Show resolved Hide resolved
@pjflux2001
Copy link
Member Author

@pjflux2001 thank you for getting this working. Looks good but I just put up a few concerns. Do we need the production folder? PolyPhy/experiments/jupyter /production/ The path looks very long. Could you also squash your commits to one issue. The conventional way should be one commit per issue fixed, this avoids us getting passed the 2GB that git provides.

Since the 3D mask jupyter notebook isn't yet working / incomplete, I checked with @OskarElek and he suggested segregating it from the other notebooks. Hence, this change in path.

@OskarElek could you please turn on squashing : https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/configuring-commit-squashing-for-pull-requests

@OskarElek OskarElek merged commit ca5297d into PolyPhyHub:main Jun 8, 2023
12 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.

Tests jupyter notebooks
3 participants