lf-lang / lf-python-support

Python package for supporting code generated by the Lingua Franca compiler
Other
5 stars 0 forks source link

Python target does not provide user-facing log functions #28

Open Soroosh129 opened 3 years ago

Soroosh129 commented 3 years ago

The C target has a few useful user-facing APIs such as DEBUG_PRINT() and error_print() that will allow the user to add thread-safe print statements in the body of reactions. The output (or lack thereof) of these APIs can then be controlled using the logging target specification.

This could be a very useful feature in the Python target as well.

To-Do

lhstrh commented 3 years ago

This ticket should probably be transfered to the reactor-c-py repo?

https://docs.github.com/en/issues/tracking-your-work-with-issues/transferring-an-issue-to-another-repository

Soroosh129 commented 3 years ago

You are right that this issue might squarely fall into the reactor-c-py category. However, I have a few concerns about moving this to that repo.

First, issues related to the Python target can get scattered around. This is not a big deal but it makes it harder to keep track of them all. Second, I worry about visibility. The satellite reactor repos are not as visible as the main repo, so finding volunteers to fix those issues becomes harder. Third, addressing this issue requires updating the wiki on the main repo. Should I create a separate issue for that?

Any thoughts on how we could increase the visibility of the issues in the smaller repos? Maybe we can pin an issue that links to the issue pages of the satellite repos?

lhstrh commented 3 years ago

I'm not sure I understand why moving issues like these makes it harder to keep track of them. We've been doing that for reactor-ts for a long time. Keeping issues close to the code that concerns them only make sense to me.

It is always possible to reference an issue in another repo, like so: https://github.com/lf-lang/reactor-c/issues/2

Either way, it was just a suggestion. If you want to keep it here, that's fine too.

Soroosh129 commented 3 years ago

Keeping issues close to the code that concerns them only make sense to me.

That's a good point. The biggest concern for me is visibility. Thoughts on this?

Maybe we can pin an issue that links to the issue pages of the satellite repos.

lhstrh commented 3 years ago

I personally don't really perceive a visibility issue. It hasn't been a problem for any of the other runtime repos that I know of...

Soroosh129 commented 3 years ago

To be honest, the issues in reactor-ts have remained mostly hidden to me until now that I have checked it after a year and a half...

lhstrh commented 3 years ago

But why would you be checking the issues if you're not actively contributing code? If you push code to that repo, you see the issues. If you don't push code, you don't see the issues. That's sounds perfectly reasonable to me.

Soroosh129 commented 3 years ago

I think the big picture goal should be to solicit new contributors that can fix issues, not to compartmentalize development into obscure places.

cmnrd commented 3 years ago

I would like to add that this feature is already implemented in a very portable, flexible and adjustable way in the widely used logging package. There is no need to reimplement this functionality (as far as I can see).

Soroosh129 commented 3 years ago

I would like to add that this feature is already implemented in a very portable, flexible and adjustable way in the widely used logging package. There is no need to reimplement this functionality (as far as I can see).

Thanks for this suggestion. Are you referring to this?

I think a nice-to-have feature that the C log functions offer (which this module doesn't by default) is that if the program is federated, they prepend a federate x: to each log message.

The logging module does look very customizable though. Maybe we can find a way to add that functionality by using custom display formats.

cmnrd commented 3 years ago

Are you referring to this?

Yes.

The logging module does look very customizable though. Maybe we can find a way to add that functionality by using custom display formats.

Indeed, this is how you would achieve this. You can easily insert a line that configures logging in a central place in the generated Python code. Also adding timestamps (physical and/or logical) is possible. In Mocasin, we additionally use a customized formatter to automatically print the current logical timestamp of the event for each message produced during simulation.