dsfranca / knowyourlimit Goto Github PK

The project lacks setup.py (or setup.cfg+pyproject.toml) and the requirements.txt are very specific.
I would be happy to help you prepare it for your project, you can take a look at how we did it for orqviz.
Having it will help with package installation and putting it on pypi later on.

You could reformat your package to be a bit more consistent with common practice for open science software in Python. A lot more information and a template is here: https://github.com/nathanshammah/make-your-code-count

You'd have a knowyourlimit module/folder at the top level that all of the code for your package goes into. There's more details in the linked template and would be happy to discuss live if that would help

I don't think the project has any unit tests.
This is bad for multiple reasons:

• it makes development and maintenance harder
• makes it more vulnerable to changes in the dependencies
• makes it much less trustworthy
• creates huge risk of accidentally breaking some functionalities

I know it's a lot of work to add them, but I'd be glad to find some people that would be willing to help with this task :)
And I'd be glad to help with designing how they should look like and sharing some good practices.

This module has some modular functions, but there's also:

• e.g. blocks of comments
• module level code: https://github.com/dsfranca/KnowYourLimit/blob/main/cirq_circuits.py#L43-L96

It wasn't clear to me that any of the examples use the module level code. Can this just be removed?

Is the code in the /recirq folder just the package https://github.com/dsfranca/KnowYourLimit/tree/main/recirq? If so, this could be included as a package dependency instead of copying over the source.

Main advantage would be that you could keep current with updates and improvements to recirq.

If there are changes needed to recirq then you could either fork the package or potentially upstream the changes to recirq itself.

Happy to open a PR to fix either way, but wanted to open up the discussion first to see what the intended use of the package is @dsfranca

It looks like there are some system files sticking around in the repo that could be removed:
.vscode is ignored in the https://github.com/dsfranca/KnowYourLimit/blob/main/.gitignore
.ipynb_checkpoints and __pycache__ can also likely be git removed.

Any reason to keep them around? @dsfranca