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

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

Open
2 tasks
Soroosh129 opened this issue Oct 16, 2021 · 11 comments
Open
2 tasks

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

Soroosh129 opened this issue Oct 16, 2021 · 11 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@Soroosh129
Copy link
Contributor

Soroosh129 commented Oct 16, 2021

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

  • Implement log functions
  • Update the wiki once this feature is implemented.
@Soroosh129 Soroosh129 added the enhancement New feature or request label Oct 16, 2021
@lhstrh
Copy link
Member

lhstrh commented Oct 17, 2021

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

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

lhstrh commented Oct 18, 2021

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: lf-lang/reactor-c#2

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

@Soroosh129
Copy link
Contributor Author

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

lhstrh commented Oct 18, 2021

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

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

lhstrh commented Oct 18, 2021

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

Soroosh129 commented Oct 18, 2021

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

@cmnrd
Copy link

cmnrd commented Oct 18, 2021

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

Soroosh129 commented Oct 18, 2021

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

cmnrd commented Oct 18, 2021

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.

@lhstrh lhstrh added the good first issue Good for newcomers label May 22, 2023
@lhstrh lhstrh transferred this issue from lf-lang/lingua-franca May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants