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

Fix colcon python warnings #3742

Closed
wants to merge 7 commits into from

Conversation

omichel
Copy link
Contributor

@omichel omichel commented Jun 19, 2023

This PR adds instructions to disable the print of colcon python warnings on stderr that are confusing users.
It implements the recommended solution explained here.

We are aiming at merging this PR to rolling, humble and iron.

Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to add explanation something like this in the doc.

instead of adding this comments before every tutorial that issues colcon build, how about adding it in the doc known issue for humble and iron release note?

@omichel
Copy link
Contributor Author

omichel commented Jun 20, 2023

I agree it doesn't hurt to add this into the known issues for humble and iron release notes.
However, my feeling is that people don't read the known issues in the release notes unless they bump into a problem.
In my view, adding these instructions to the tutorials provides a much smoother user experience.
Some developer complained about this problem and the solution provided here fully satisfies them.

@omichel omichel requested a review from fujitatomoya June 20, 2023 07:21
@clalancette
Copy link
Contributor

I have to say that I'm reluctant to put this into the official documentation, especially now that we have a solution on the horizon: https://discourse.ros.org/t/call-for-testing-standards-based-python-packaging-with-colcon/32008

@omichel
Copy link
Contributor Author

omichel commented Jun 21, 2023

I will be happy to remove it as soon as the solution is effective.
However, right now it's a pain for all users and we should do something, otherwise here is what is happening:

All users bump into this problem and they react differently:

  1. Some (a small minority) will report it to us and we will have to answer them (takes our precious time).
  2. Some will search the web, find a solution on their own, test it, implement it (takes their precious time).
  3. Some will ignore it or simply give up, with the feeling of a poor quality software.

Do we really want this?

@clalancette
Copy link
Contributor

Do we really want this?

I agree it isn't great, but as soon as we add this type of thing to the official documentation, people will copy it everywhere. And it has the side-effect of hiding these warnings for all Python packages, whether they are part of ROS or not. So I still think it is better to fix it at the source (as we are doing with colcon-python-project above).

@omichel
Copy link
Contributor Author

omichel commented Jun 21, 2023

I fully agree it is better to fix it at the source. But I fear this will take long and meanwhile users are getting confused.
Do you have a estimate on when this will be integrated into humble, iron and rolling?

@clalancette
Copy link
Contributor

Do you have a estimate on when this will be integrated into humble, iron and rolling?

At the moment, I do not. But I hope it is soon.

@omichel
Copy link
Contributor Author

omichel commented Jun 27, 2023

OK, so this problem will remain unresolved for an undefined amount of time...

To me, this is a very bad approach and gives a poor impression on the quality of the ROS 2 software and its documentation.

USE CASE: As a new robotics developer, I want to try out ROS 2.

  1. I search the web and find the official tutorials.
  2. I follow them with the latest, most stable version of ROS 2.
  3. When compiling packages from the source, I am getting plenty of errors flooding my console.
  4. From there, several possible cases:
    4.1 Everything seems to work as expected, so I don't bother, but have a feeling that the quality of the software is very poor and something may go wrong sooner or later.
    4.2 Something doesn't work and I suspect it may be the result of the errors I just got. This is misleading.
    4.3 I search the web and find this long discussion, so I can safely ignore the problem, but I lost quite some time.

In all the cases, the results is a very poor developer experience with the feeling that the quality of the ROS 2 software and/or documentation is low.

I believe that fixing the official documentation would resolve the problem, even though I agree that it may have some side effects, like people creating video tutorials with these instructions that will become obsolete after the problem is patched. But this is generally the case for any outdated tutorial, so I am pretty sure we should not bother about this.

And of course, we should fix the official documentation as soon as the patch reaches the official distributions.

@clalancette
Copy link
Contributor

So, it turns out that https://discourse.ros.org/t/call-for-testing-standards-based-python-packaging-with-colcon/32008 isn't going to be ready for Jazzy. While that is still the correct way forward, for now we are going to suppress the warning within colcon: colcon/colcon-core#626. The benefit there is that it restricts the suppression to only things built with colcon, not the whole system .

So with that said, I'm going to close this out.

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.

3 participants