feat: use own logger, not global logger

[kroeschl]

Description

Instead of calling logging.debug() (or similar) directly and causing a call to logging.basicConfig(), log via our logger only. Also stop manipulating the logging level for our module and let users manage that. Fixes #9.

Testing

poetry run tox succeeds. Also ran the example from the issue and saw no output:

import logging
import serializable

@serializable.serializable_class
class Foo:
    pass

_logger = logging.getLogger()
_logger.addHandler(logging.NullHandler())
_logger.error("foo")

[jkowalleck]

Feature looks promising.
some tests are failing. could you get back when they pass?

[jkowalleck]

@kroeschl i pushed an update to your proposal: d5f59ef

[jkowalleck]

IC is failing.
I will sort that out via #17 so this feature can be releases ASAP.

[jkowalleck]

@kroeschl i think all is settled,
could you do a review and see if the feature is still as you expected it?

[kroeschl]

@kroeschl i think all is settled, could you do a review and see if the feature is still as you expected it?

This PR is now a bit more than what I intended to change. Specifically:

• Converting existing warnings to log messages is IMO a design decision outside the scope of this change.
• Using a logger name of serializable._logging.LOGGER is counter to Python logging convention and may confuse users. Also, a logger name including variations of "logging" is somewhat redundant.
• Generally, adding a logging module is probably overkill since we're not doing anything special in it (aside from the warning stacklevel change). Users can already configure logging in their own code by using getLogger("serializable") (or whatever name).
• We shouldn't set a default logging level. That should be left up to the user.

I'm inclined to roll this back to just the fix for the issue and let other rework live in a separate PR.

[jkowalleck]

thanks for looking into this.

* Converting existing warnings to log messages is IMO a design decision outside the scope of this change.

From a architectural perspective, it looked obvious to move all log/output to an own logger.
Warnings were sent to global warning handler, right? So better have them to a logger that you can control downstream.
Would you agree?

* Using a logger name of `serializable._logging.LOGGER` is counter to [Python logging convention](https://docs.python.org/3/library/logging.html#logger-objects) and may confuse users. 

Also, a logger name including variations of "logging" is somewhat redundant.

"Python logging convention"? If there is no PEP, then there is no convention, right?
The linked documentation uses just some example to help people chose a unique name for a logger.
And most importantly, accessing the logger is possible via public API from serializable import LOGGER, so there is no need to know the name in the first place, or is there?
What do I not see? There must be something I missed.

* Generally, adding a logging module is probably overkill since we're not doing anything special in it (aside from the warning `stacklevel` change). [...]

It was needed to prevent circular dependencies ;-)

* [...] Users can already configure logging in their own code by using `getLogger("serializable")` (or whatever name).

The mentioned module is nonpublic API. And so the name is nonpublic.
Should the name be dropped, to make this obvious?

* We shouldn't set a default logging level. That should be left up to the user.

You see that the logger is a global one, so there are possibly multiple "users".
Downstream users would need to add logHandlers in the first place, so they would do log level filtering on the handler.
Let's say library A and B both uses serializable, and A wants to listen to Errors only, B wants to listen to warnings and above, then you proposed to:
each library sets the log level on the logger. This would end with a situation that the last setter wins.
In contrast, the current solution is that the logger initiates with the lowest level it is used with by this very lib, and log handlers apply filters
like in the documentation you must have seen: https://github.com/madpah/serializable/blob/1b39831011b018fca1c3bd0454cc7788f168ebdd/docs/logging.rst?plain=1
Is there something wrong with this approach?

[jkowalleck]

❌
bring back logger and mark it as deprecated.
even though logger was not used for any logging in the past, it still was public API.

but all UPPERCASE is preferred to point downstream users to the fact, that is a readonly/constant value

[jkowalleck]

or leave as is and consider it a breaking change -- which is okay for a planned v1.0.0

[jkowalleck]

outdated, superseded by #47