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

cannot name the logger object "logger" #9

Open
ruck94301 opened this issue Aug 15, 2022 · 5 comments
Open

cannot name the logger object "logger" #9

ruck94301 opened this issue Aug 15, 2022 · 5 comments
Assignees
Labels
bug Something isn't working documentation
Milestone

Comments

@ruck94301
Copy link

ruck94301 commented Aug 15, 2022

In UserGuide.md,
The suggested usage statement on line 96 seems like a documentation error...

95  ```matlab
96  logger = logger.Logger.getLogger('foo.bar.baz.MyThing');
97  logger.info('Something happened');
98  ```

If it worked, then the logger would henceforth refer to the logger object instead of the "package", I think.
But it doesn't work, it produces an error:
Undefined function or variable 'logger'.
(in R2015b, at least)

This works:

mylogger = logger.Logger.getLogger('foo.bar.baz.MyThing');
mylogger.info('Something happened')
@apjanke
Copy link
Member

apjanke commented Aug 15, 2022

Yep, that's a flaw in the documentation. I think it's left over from when I was using a different name for the top-level package/namespace, and then did a global search-and-replace without closely re-reading all the example code. You shouldn't, and maybe can't, use the same name for both a local variable and a package, though I'm not sure exactly what errors it'll cause.

I'll come up with a better variable name and update the examples here.

@apjanke apjanke self-assigned this Aug 15, 2022
@apjanke apjanke added bug Something isn't working documentation labels Aug 15, 2022
@apjanke apjanke added this to the 1.4.0 milestone Aug 15, 2022
@apjanke
Copy link
Member

apjanke commented Aug 17, 2022

Okay, you know what: logger is such an obvious and good name for a variable holding a logger object, maybe we should just rename SLF5M's top-level package from +logger to +logging? That sounds better as a "topic area" name anyway.

SLF4M has few enough users at this point that we can take a breaking change to the interface.

@ruck94301
Copy link
Author

My background with logging frameworks is only from python standard logging framework. I have no direct experience with log4j, but I believe there is a lot of similarity. Hopefully my python background explains my mental model and the prism through which I am seeing things.
A name change to +logging is consistent with python. It's not necessary for me, but it does make it easier for someone like me. So I support the name change if it doesn't create too much trouble for other stakeholders.

@ruck94301
Copy link
Author

In python, we use the package function like

logging.info('milestone')  # instead of SLF4M's logger.info()

and use a logger object method like

logger = logging.getLogger(__name__)  # instead of SLF4M's logger.Logger.getLogger()
logger.info('milestone')

@apjanke
Copy link
Member

apjanke commented Aug 28, 2022

My background with logging frameworks is only from python standard logging framework. I have no direct experience with log4j, but I believe there is a lot of similarity. Hopefully my python background explains my mental model and the prism through which I am seeing things.

Your background with these other logging packages is relevant; they all work pretty similarly, and SLF4M is explicitly modeled on them. The issue here is actually one specifically with Matlab's syntax for M-code and packages: in Matlab, package or namespace prefix names, and the prefixes for package-qualifying an identifier name in a function or class reference, effectively share a namespace with local variables, and syntactically look exactly like object or struct field references on local variables (and some other things), and they can collide, and when they do, sometimes they produce error conditions that are non-obvious. In Java, its syntax has a distinct form for package qualification, which makes package names and local variables separate namespaces, so they can't collide at all. And in Python, module imports effectively are variables within another module, so it's a single (local) namespace and you can have collisions, but they're more obvious, and Python developers will know how to work around them, because that is a normal and common way for things to work in Python. But in Matlab, the whole concept of packages/namespaces seems to be an advanced and lesser-used technique.

So, your background with other logging frameworks is probably entirely sufficient here and your mental model is correct, and you're just running in to some Advanced Matlab-Specific Syntax Shizz here.

A name change to +logging is consistent with python. It's not necessary for me, but it does make it easier for someone like me. So I support the name change if it doesn't create too much trouble for other stakeholders.

I think SLF4M has a total of like 4 active users at the moment, so the upper limit on how much trouble API changes could cause is pretty small. :)

I am leaning toward making this name change. It just seems like a much better naming arrangement to me. Especially because:

...and use a logger object method like

logger = logging.getLogger(__name__)  # instead of SLF4M's logger.Logger.getLogger()
logger.info('milestone')

seems like a totally reasonable and useful/developer-friendly thing to do. And having the exact same name as Python's logging module would be a user-friendly thing to do that makes it easier to discover SLF4M's code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation
Projects
None yet
Development

No branches or pull requests

2 participants