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

Improve test_problems.py #226

Closed
vishal-ph opened this issue Apr 12, 2023 · 15 comments
Closed

Improve test_problems.py #226

vishal-ph opened this issue Apr 12, 2023 · 15 comments
Assignees
Labels
good first issue Good for newcomers tests Everything related to unit-testing

Comments

@vishal-ph
Copy link
Collaborator

vishal-ph commented Apr 12, 2023

Issue Description

  • The file is too long and contains tests for all problem classes. We need to break it down as separate files testing each problem class for ease of maintainability.
  • The tests contain some unused code and non consistent coding standards that need to be fixed.

Describe the solution you'd like:

  • Break down the file src/openqaoa-core/tests/test_problems.py into different files containing relevant tests for each problem class defined in OpenQAOA
  • OpenQAOA problem classes are defined in src/openqaoa-core/openqaoa_core/problems/.

Create a new folder in test_problems and create scripts consisting of specific tests corresponding to that problem class.
For e.g. create test_maximumcut.py and put all tests corresponding to maximum_cut.py in this file

NOTE: Please branch out of openqaoa/dev and make the PR to openqaoa/dev

@vishal-ph vishal-ph added good first issue Good for newcomers tests Everything related to unit-testing labels Apr 12, 2023
@vishal-ph vishal-ph added unitary_fund A feature supported by an UF grant and removed unitary_fund A feature supported by an UF grant labels May 17, 2023
@jiggychauhi
Copy link

Hey, this sound like something a newbie like me can do. Please assign me this issue.

@devilkiller-ag
Copy link

Hi, Can I work on this issue?

@vishal-ph
Copy link
Collaborator Author

Hello, thanks for your interest in contributing to OpenQAOA!
We encourage you to collaborate on this issue, otherwise, feel free to make an attempt individually and raise a PR to openqaoa/dev branch.

@devilkiller-ag
Copy link

Thank you @vishal-ph, Since it's a good first issue I will like to try it alone.

This was referenced May 29, 2023
Closed
@devilkiller-ag
Copy link

Hi @vishal-ph, can you check and confirm if I am using correct naming conventions for test files or not?
Also the example 01_workflows_example.ipynb is using MaximumCut and NumberPartition both. Under which problem test file should I keep it in?

@devilkiller-ag
Copy link

@vishal-ph , I have moved all the tests into different files. Kindly heck my PR if it's ready for merge with dev branch or not.

@devilkiller-ag
Copy link

@vishal-ph any updates?

@vishal-ph
Copy link
Collaborator Author

@devilkiller-ag, I have added my comments on the PR itself

@Newtech66
Copy link
Contributor

@vishal-ph The path is tests/test_problems.py right, not src/openqaoa-core/tests/test_problems.py?
So, we want it to be broken into tests/test_problems/test_*.py? (where * is a problem class)

@devilkiller-ag
Copy link

devilkiller-ag commented Jun 13, 2023

@vishal-ph the hyphen in folder name 'openqaoa-core' is causing an import problem. Python import statemens follows standard naming conventions, where module names shoukd consist of letters, digits and underscores only. Can I rename it to 'openqaoa_core'?

@vishal-ph
Copy link
Collaborator Author

@Newtech66, no, the path is correct. You will need to look at the dev branch. Note the comment at the end of issue description

@vishal-ph
Copy link
Collaborator Author

vishal-ph commented Jun 13, 2023

@devilkiller-ag, yes, you are right about the import name conventions. However, openqaoa-core is just the name of the module, the import statement still works with openqaoa. Similarly for the remaining plug-ins, they are imported with underscores in the names. For instance, openqaoa-qiskit will be imported as openqaoa_qiskit.
So, please refrain from implementing any name changes

@Newtech66
Copy link
Contributor

@vishal-ph Ok, so all the new files should live in the same path as test_problems.py? Not a subfolder ./test_problems/ right?

@vishal-ph
Copy link
Collaborator Author

vishal-ph commented Jun 13, 2023

@Newtech66 Yes, that's correct. Although eventually, we would want to put all the problem class tests in the test_problems folder as you suggested. This would then require updating the test workflows, so that can be done later.

@vishal-ph
Copy link
Collaborator Author

Fixed in PR #254

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers tests Everything related to unit-testing
Projects
None yet
Development

No branches or pull requests

4 participants