Skip to content

Split main and add test#1

Merged
sbryngelson merged 10 commits into
masterfrom
refac
May 13, 2025
Merged

Split main and add test#1
sbryngelson merged 10 commits into
masterfrom
refac

Conversation

@NicoRenaud
Copy link
Copy Markdown
Collaborator

Hey @kaminotesf I've implemented the suggestions I've made my emails:

  1. I've split main.py into two files, one for the tree/forest and one for the sampling of the circuits.
  2. I've started some basic test

I'll let you know as soon as the PR is ready for review !

@NicoRenaud NicoRenaud marked this pull request as ready for review May 12, 2025 13:48
@NicoRenaud NicoRenaud requested review from kaminotesf and sbryngelson and removed request for kaminotesf and sbryngelson May 12, 2025 14:20
@NicoRenaud
Copy link
Copy Markdown
Collaborator Author

@kaminotesf I think it should be good enough for a first review. Just a few additions on top of what I've already mentioned:

  • I've removed a few unused imports and fixed a couple typehints
  • I've slightly modified get_samples() so that it can accepts Aer V1 and runtime V2 samplers. If we want to accept more samplers we should find a more elegant solution
  • The tests simply check that the functions run. If we want to compare the output to predetermined results we still have to do that.
  • I've modified the CI so that the tests are automatically ran. If we want to link to coveralls (https://coveralls.io/) someone on your side needs to allow coveralls to have access to the repo. This can be done for free once the repo is open. We can then add a badge to the repo to show the coverage.
  • I didn't run the notebooks but I think they should still work

@sbryngelson
Copy link
Copy Markdown
Member

@NicoRenaud added coveralls and badge

@sbryngelson sbryngelson merged commit cbab6a6 into master May 13, 2025
7 checks passed
@sbryngelson sbryngelson deleted the refac branch May 13, 2025 06:13
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