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

Remove internal usage of pycromanager.logging #769

Merged
merged 1 commit into from
Jun 7, 2024

Conversation

bruno-digitbio
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Member

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

@henrypinkard
Copy link
Member

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
Copy link
Contributor Author

@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!

This removes public API (first introduced in micro-manager#719) for bypassing
Python's internal logging system by creating and caching a logger
instance in place of using `logging.getLogger("pycromanager")`.

Users of that previous API---i.e., application code authors that want to
globally control logging and users that want to customize Pycromanager's
logging outputs---are encouraged to simply directly use the logging
module to affect these changes instead. See micro-manager#766 for discussion.
@henrypinkard henrypinkard merged commit 7bbf5b4 into micro-manager:main Jun 7, 2024
2 checks passed
@henrypinkard
Copy link
Member

Thanks for all your work on this!

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