micro-manager / pycro-manager

Python control of micro-manager for customized data acquisition
https://pycro-manager.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
165 stars 52 forks source link

Remove internal usage of `pycromanager.logging` #769

Closed bruno-digitbio closed 5 months ago

bruno-digitbio commented 5 months ago

As described in #766, here we move towards a more canonical use of Python's logging module by having pycromanager sub-modules define their own loggers and use them.

We cannot remove the existing monkey-patch-based pycromanager.logging module yet, since it is public API surface and is used by PyJavaZ, and therefore it must be kept until at least those references are removed, but I went ahead and nuked the relevant documentation...happy to additionally (or instead) add any deprecation or other warnings to the corresponding functions to try to prevent future use until they are removed?

bruno-digitbio commented 5 months ago

Note that this PR was initially drafted based on the following search:

$ grep -R logging . | cut -d':' -f1 | sort | uniq | grep -v '\.git' | grep -v "Binary file"
./java/src/main/java/org/micromanager/remote/RemoteViewerStorageAdapter.java
./pycromanager.egg-info/SOURCES.txt
./pycromanager/acquisition/java_backend_acquisitions.py
./pycromanager/headless.py
./pycromanager/logging.py
henrypinkard commented 5 months ago

Bumped to the new version of pyjavaz so you should be good to build on that

henrypinkard commented 5 months ago

All set to merge this? The only other thing I see is to bump the minor version in pycromanager/_version.py. The new version will automatically build and deploy to pypi

bruno-digitbio commented 5 months ago

@henrypinkard , should be good-to-go! Just wanted to spot-check it on an actual computer with a scope to make sure there were no unruly typos or something silly.

This is technically a backwards-compatibility-breaking change (since pycromanger.logging was public), but I went ahead and bumped the "minor" number since the "major" number is still zero. Feel free to edit-in-place the versioning bump as needed before merging!

henrypinkard commented 5 months ago

Thanks for all your work on this!