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

Dev.logs #75

Merged
merged 1 commit into from
Sep 12, 2023
Merged

Dev.logs #75

merged 1 commit into from
Sep 12, 2023

Conversation

roedoejet
Copy link
Member

Refactored default save_directory for logs and model checkpoints to logs_and_checkpoints instead of logs. Fixes #39

@roedoejet roedoejet changed the base branch from main to dev.docs September 7, 2023 21:30
@codecov
Copy link

codecov bot commented Sep 7, 2023

Codecov Report

Merging #75 (5db4ec3) into main (9d6fa75) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #75      +/-   ##
==========================================
+ Coverage   52.92%   52.96%   +0.03%     
==========================================
  Files          43       43              
  Lines        2564     2566       +2     
  Branches      350      350              
==========================================
+ Hits         1357     1359       +2     
  Misses       1138     1138              
  Partials       69       69              
Files Changed Coverage Δ
everyvoice/config/shared_types.py 84.48% <100.00%> (ø)
everyvoice/tests/test_wizard.py 94.00% <100.00%> (+0.12%) ⬆️
everyvoice/wizard/basic.py 74.40% <100.00%> (+0.20%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@joanise joanise removed their request for review September 8, 2023 16:13
@joanise
Copy link
Member

joanise commented Sep 8, 2023

I'll let @SamuelLarkin or @marctessier review this, I have no opinion on this one.

Base automatically changed from dev.docs to main September 8, 2023 17:32
@SamuelLarkin
Copy link
Collaborator

Humm can the branch be rebased because the diff is showing none related changes?

@joanise
Copy link
Member

joanise commented Sep 11, 2023

@SamuelLarkin I'll rebase it shortly.

@joanise
Copy link
Member

joanise commented Sep 11, 2023

@SamuelLarkin Rebased, this should be easy to review now.

@@ -2,6 +2,7 @@
__pycache__
.coverage
logs
Copy link
Collaborator

@SamuelLarkin SamuelLarkin Sep 12, 2023

Choose a reason for hiding this comment

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

Shouldn't we drop logs if we renamed it to logs_and_checkpoints?

Copy link
Collaborator

@SamuelLarkin SamuelLarkin left a comment

Choose a reason for hiding this comment

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

Looks sound.

@SamuelLarkin SamuelLarkin merged commit e05ac37 into main Sep 12, 2023
4 checks passed
@SamuelLarkin SamuelLarkin deleted the dev.logs branch September 12, 2023 15:48
@roedoejet
Copy link
Member Author

roedoejet commented Sep 12, 2023

Thanks @SamuelLarkin ! For small PRs like this, please rebase instead of creating a merge commit. Thanks!

@roedoejet
Copy link
Member Author

Thanks @SamuelLarkin ! For small PRs like this, please rebase instead of creating a merge commit. Thanks!

Nevermind! I see that you did that! For some reason the app makes it look like you did a merge commit, but I see on the desktop that you rebased. Sorry for the noise!

@joanise
Copy link
Member

joanise commented Sep 12, 2023

Well, rebasing this one means we have to rebase all the ones after. I might have taken the time to do a fast-forward merge from the CLI for this one, for that reason, but this is fine too.

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.

Why are the models under logs/?
3 participants