[SofaPython] Fix python live coding that is broken #414
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I took me a while to understand that writing PythonEnvironement::gil lock(); While perfectly compiling is not doing what I expect. I wonder how to help developpers no to make this mistake.
When the same path is added two time one its is absolute forme one in relative. The FileMonitor does not work on the absolute one. This Commit fix that.
damienmarchal
added
pr: fast merge
|
@damienmarchal I understand the problem can go unnoticed since the value is never used elsewhere and won't get to trigger a type error down the road, so removing the default value for the constructor argument seems like a reasonable (and portable) fix. |
This was referenced Oct 1, 2017
This was referenced Oct 5, 2017
guparan
added
pr: status ready
|
@hugtalbot, @guparan This one should be merged. |
|
[ci-build] |
|
This one seems ready. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi,
This PR fix the live coding of python component. I found two problems.
One is a bug in the FileMonitor which prevent to monitor several files located in the same directory but given in a different manner as in:
The second problem is a missing PythonEnvironement::gil before calling python code.
This PR fix the two.
@guparan and @hugtalbot Could this PR be treated in fast-path please ?
@maxime-tournier I took me a while to understand that
PythonEnvironment::gil lock();Is not doing anything useful. The problem is well known but in general it is quickly catched but here, as lock is a RAII, everything compile but just crash when calling python code. I'm quite sure this will happen to a lot of people so I wonder if we can make something more elegant that removing the default constructor and forcing to provide the *trace pointer (possibly nullptr). Any idea ?
This PR:
Reviewers will merge only if all these checks are true.