Skip to content

Johan/devbranch#29

Merged
c-salomonsen merged 15 commits intomainfrom
johan/devbranch
Feb 4, 2025
Merged

Johan/devbranch#29
c-salomonsen merged 15 commits intomainfrom
johan/devbranch

Conversation

@Johanmkr
Copy link
Contributor

@Johanmkr Johanmkr commented Feb 4, 2025

Implemented model and metric.

@Johanmkr Johanmkr requested a review from c-salomonsen February 4, 2025 09:35
@c-salomonsen
Copy link
Contributor

c-salomonsen commented Feb 4, 2025

Nice! Looks very good. I just have a quick remark:

Solveig set up a tests directory. If you put your tests somewhere in there, for instance those related to metrics in tests/test_metrics.py they will be run automatically when you push towards the main branch.

c-salomonsen
c-salomonsen approved these changes Feb 4, 2025
View reviewed changes
Copy link
Contributor

@c-salomonsen c-salomonsen left a comment

Choose a reason for hiding this comment

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

NIcely done. Approving

@c-salomonsen c-salomonsen self-requested a review February 4, 2025 12:11
c-salomonsen
c-salomonsen requested changes Feb 4, 2025
View reviewed changes
Copy link
Contributor

@c-salomonsen c-salomonsen left a comment

Choose a reason for hiding this comment

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

The formatting is failing. Can you format then push again?

@c-salomonsen c-salomonsen self-requested a review February 4, 2025 12:14
c-salomonsen
c-salomonsen approved these changes Feb 4, 2025
View reviewed changes
Copy link
Contributor

@c-salomonsen c-salomonsen left a comment

Choose a reason for hiding this comment

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

Looks good. Ready to merge

@c-salomonsen c-salomonsen merged commit a5a6c01 into main Feb 4, 2025
4 checks passed
@Johanmkr
Copy link
Contributor Author

Johanmkr commented Feb 4, 2025

Nice! Looks very good. I just have a quick remark:

Solveig set up a tests directory. If you put your tests somewhere in there, for instance those related to metrics in tests/test_metrics.py they will be run automatically when you push towards the main branch.

Yes i will move the tests into test, thanks for reminding me @salomaestro.

@c-salomonsen
Copy link
Contributor

c-salomonsen commented Feb 4, 2025

Ah I forgot about that, sorry. I think you can revert the merge if you want to have those changes in this PR, if not, just make a new one.

@hzavadil98 hzavadil98 deleted the johan/devbranch branch February 7, 2025 10:15
Seilmast pushed a commit that referenced this pull request Feb 7, 2025
@c-salomonsen
Merge pull request #29 from SFI-Visual-Intelligence/johan/devbranch
ca29835 
Johan/devbranch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@c-salomonsen c-salomonsen c-salomonsen approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Johanmkr @c-salomonsen