STEPbySTEPproj / Protocol_biomechanics

0 stars 1 forks source link

switch from crlf to lf end of file #19

Closed aremazeilles closed 2 years ago

aremazeilles commented 2 years ago

We had this pending since some time.

There are 2 C processes that fails. This MR handles the yaml-lint issue

This is due to the fact that windows and linux are not using the same end of line encoding.

If I look at the initial file state, I see, in Visual Code:

Screenshot from 2021-10-21 15-45-46 Look at bottom right: CRLF is written. Windows style, but we do all CI under Linux.

I changed it to be LF (just by clicking on this text in Visual Code), and I get:

Screenshot from 2021-10-21 15-48-50

And this makes the CI process not complain about it,

It would be great if you could, before uploading yaml files from windows, to make this change. Several options to do it are available, see here

The other issue, not yet solved, is related to this trial.

Here we generate the docker image of your code, and launch it using the inut reference file, double checkign that the generated output is the same. And it is not. I usually launch the process, and then compare the generated output folder, with the reference one stored in the repo. Using meld (winmerge is I think an alternative in Windows):

Screenshot from 2021-10-21 15-58-53

Left is the generated, right is what is in the reference output folder. On the one side, Files Foot(1,2).{Begin,End}{Stance, Swing}.yml are not present into the reference output folder.

This is where complains the CI process:

Traceback (most recent call last):
  File "test_docker_call.py", line 127, in test_call_docker
    self.assertCountEqual(output_files, output_files_expected, msg="Missing generated files")
AssertionError: Element counts were not equal:
First has 1, Second has 0:  'Foot(2).EndSwing.yml'
First has 1, Second has 0:  'Foot(1).BeginSwing.yml'
First has 1, Second has 0:  'Foot(2).EndStance.yml'
First has 1, Second has 0:  'Foot(1).BeginStance.yml'
First has 1, Second has 0:  'Foot(1).EndStance.yml'
First has 1, Second has 0:  'Foot(2).BeginStance.yml'
First has 1, Second has 0:  'Foot(2).BeginSwing.yml'
First has 1, Second has 0:  'Foot(1).EndSwing.yml' : Missing generated files

Then, other files differ. For the first one for instance, I have: On the left (generated) :

type: scalar
value: 0.2

while on the right (reference):

type: scalar
value: 0.19594594594594594

which is not the same. So I would like you to check these differences as well.

Then maybe we should discuss about the added PIs, Foot(1,2).{Begin,End}{Stance, Swing}.yml The filename is not compliant with the pi filename (no space). Then all are defined as vector, but they just have a single value (can accept it, but to be checked with you). On my PC, all Foot(2) measures are even empty, and I don know if this is normal.

To resume, I would suggest to:

What do you think? We need to solve this before I can launch the deployment of all these changes to the server

dborro commented 2 years ago

Hi Anthony, Concerning to CRLF, we will change it manually (as we work in Windows). Concerning to files Foot(1,2).{Begin,End}{Stance, Swing}.yml,...; firstly, I have to say that those files are not metrics themselves. We create them because they content information that Marco needs for his EMG sync. What can we handle this? Maybe we can save those files in another folder so your code can work?

aremazeilles commented 2 years ago

Hi Diego. I had a quick check, and it seems that the change through Visual Code is possible as well under windows (actually, this is how I do it under linux, i.e file per file manually).

For the foot files, uploading them to the reference output folder should be enough to get the CI process happy (if this is a generated file, we can verify that the generated file remains the same). The docker testing basically compares all files present in the folder, there is no notion of PI at this level.

i will quickly check with Alfonso whether this could affect or not the automation placed on the top of the algorithm on the server.

dborro commented 2 years ago

Hi Anthony,

aremazeilles commented 2 years ago

Hello Diego. The yaml part seems ok, thanks for the effort. About the docker test, this is strange because I effectively see the file present in the output folder. I will make some tests this afternoon, and get back to you.

aremazeilles commented 2 years ago

Ok, well I think the process is actually now fine.

If you look at the root page, the CI process is green.

The failing process is in the current branch, solve_ci, that I started to handle your previous issue, and from where all this dicussion started. But the latest changes you uploaded to the main branch makes the CI process happy.

So I would basically suggest closing this MR, and remove the associated branch, as the problem has been aside directly handled in the main branch

dborro commented 2 years ago

Ok, great.