NanoComp / imageruler

measure minimum solid/void lengthscales in binary image
https://nanocomp.github.io/imageruler/
MIT License
13 stars 6 forks source link

Reorganize into standard Python package structure and add CI #3

Closed ianwilliamson closed 1 year ago

ianwilliamson commented 1 year ago

Please advise on some of the setup.py metatdata, namely author and author email.

ianwilliamson commented 1 year ago

Addresses #2

mawc2019 commented 1 year ago

I am preparing a PR in which a new Python file is added for testing whether duality relations $\mathcal{E}_d(\bar\rho)=\overline{\mathcal{D}_d(\rho)}$, $\mathcal{D}_d(\bar\rho)=\overline{\mathcal{E}_d(\rho)}$, $\mathcal{O}_d(\bar\rho)=\overline{\mathcal{C}_d(\rho)}$, and $\mathcal{C}_d(\bar\rho)=\overline{\mathcal{O}_d(\rho)}$ hold strictly. Therefore, imageruler will have two Python files for tests. Perhaps a new folder tests should be created for them. Should I put tests outside the folder imageruler? Another relevant issue is that the two Python test files and examples.ipynb need to import regular_shapes.py. To organize the files properly, should I put regular_shapes.py in the tests folder, or create a new folder examples that contains both examples.ipynb and regular_shapes.py? I prefer the second option because those regular shapes are also examples of shapes.

ianwilliamson commented 1 year ago

If the new test case(s) are testing the functions in ruler.py, I would just add them to the existing test_ruler.py. Rationale is that for each <module_name>.py, you have a corresponding test_<module_name>.py which provides the primary test coverage for that module.

mawc2019 commented 1 year ago

If the new test case(s) are testing the functions in ruler.py, I would just add them to the existing test_ruler.py.

Yes, the new test cases are testing morphological functions in ruler.py (which will be renamed as imageruler.py), but the new and old test cases focus on different aspects.

Rationale is that for each <module_name>.py, you have a corresponding test_<module_name>.py which provides the primary test coverage for that module.

This makes sense to me, but I also saw other packages use different test files to test different features of the same module, e.g., grcwa (in which test_kbloch.py and test_rcwa.py exist) and perhaps meep (in which several test_adjoint_<feature_name>.py files exist). Which is more suitable for imageruler?

ianwilliamson commented 1 year ago

Which is more suitable for imageruler?

Up to you, ultimately. There are certainly many possibilities. Another option is to create additional classes within a single test_<module>.py file to aide with organization. e.g. we currently have TestRuler and you could add a TestDuality class with additional cases. Any class with a Test at the beginning of its name will be treated as a test.