report on repo #56

abhicodes369 · 2024-03-08T17:31:01Z

Describe the changes you have made in this PR

created a report on the repository like the name of the contributor and how many are being tested
A clear and concise description of what you want to happen
a simple HTML is being generated if you execute this script

Link this PR to an issue [optional]

Fixes #47

Checklist

Self-review of changed code
Added automated tests where applicable
Update Docs & Guides

etpeterson · 2024-03-08T23:43:08Z

combined_report.html

This should go into the website folder. Another PR is also creating files there.

etpeterson · 2024-03-08T23:45:29Z

local.env.example

What is the idea behind setting these variables? You should be able to get the repository location from the location of the file, e.g. using the __file__ variable.

etpeterson · 2024-03-08T23:46:46Z

repostatus.py

This should move somewhere else. Probably utilities/repository or something.

etpeterson · 2024-03-08T23:47:15Z

repostatus.py

+import json
+
+# Load environment variables
+load_dotenv(r'C:\Users\home\tf2.4\TF2.4_IVIM-MRI_CodeCollection\local.env')


We should be able to get around hard coding paths. This won't work for me, for example.

etpeterson · 2024-03-08T23:55:17Z

repostatus.py

+load_dotenv(r'C:\Users\home\tf2.4\TF2.4_IVIM-MRI_CodeCollection\local.env')
+
+# Read the CSV file
+csv_file_path = os.getenv('CODE_CONTRIBUTIONS_FILE')


Looking through the code I realize that your table is missing a column, wrapped. In the issue thread I mentioned parsing the files in the folders which isn't being done here, and that's what is missing. It should parse the wrapper folder to find which algorithms are wrapped.

And there I also mentioned double checking the code contributions against what is in the source folder.

So the source code folder and code contribution file should match. If not it would be nice to note that. That should be a superset of the wrapped code list. And that should be a superset of the tested code list. If anything doesn't match that logic then we should note it in the html or printing a message.

etpeterson · 2024-03-09T23:43:37Z

repostatus.py

@@ -0,0 +1,37 @@
+import os
+import pandas as pd


Pandas caused the test failure. It should be in the requirements.

abhicodes369 · 2024-03-10T16:41:22Z

I apologize I accidentally deleted the git ignore file can I create a new branch

etpeterson · 2024-03-10T16:54:55Z

I apologize I accidentally deleted the git ignore file can I create a new branch

Sure, start over or just add it back on this branch, either one.

etpeterson

Sorry, I'm rethinking this a little. Some of these that you're trying to find could be useful in the csv file anyway, so perhaps just adding them could simplify the script a lot. Then it could be a simpler process to generate the html from the csv.

etpeterson · 2024-03-10T17:56:20Z

utilities/repostatus.py

+import json
+
+# directory of the current script
+script_dir = os.path.dirname(os.path.abspath(__file__))


I think by the convention you're been following, this should be SCRIPT_DIR.

Would it be problematic to add a column called 'algorithms' to the data frame? it would be easily understandable. Is it a bad idea?

What would that add? The algorithms from the json are the ones running the automated testing. That's beyond the scope of the csv file. Is there some other algorithm you want to add?

etpeterson · 2024-03-10T18:19:47Z

utilities/repostatus.py

+        print(f"Warning: Subfolder '{subfolder}' does not exist in the source folder.")
+
+# Add column 'Tested' to the DataFrame if it starts with that of subfolder
+df['Tested'] = df.apply(lambda row: 'Yes' if any(algorithm.startswith(row['subfolder'].split('_')[0]) for algorithm in all_algorithms) else 'No', axis=1)


I'm just looking at this logic and it seems like it's just matching everything before the first _. That's not specific because that's usually the author's initials and they can have several algorithms.

Looking at it a little more, this could become fancier, but maybe we actually want this column in our csv file. So another approach would be to just manually go through and find the matches and add a "Wrapped" column to the csv file that contains what you'd see in the algorithms.json.

Then that would reduce what is needed in here to just having a sanity check that the wrapped folders match the csv. And it would make checking for tested much easier because it would be a simple string match.

How does that sound?

do i need to manually update the code_contribution_csv file or write a script to read algorithms from algorithms.json and write 'wrapped' in csv file and automatically update whenever there is change in algorithms. json file

Add the column manually. That column is not the same as what's in algorithm.json. That column would be the wrapped name, so if an algorithm is wrapped it should have an entry there. The algorithm.json is what is being tested. In theory everything wrapped should be tested but it does need to be added and confirmed it doesn't fail. So they're not the same thing.

etpeterson · 2024-03-10T18:20:19Z

utilities/repostatus.py

+
+# Parse files in the WRAPPED_FOLDER 
+wrapped_algorithms = []
+for root, dirs, files in os.walk(WRAPPED_FOLDER):


This could just be added to the csv, see comment above.

etpeterson · 2024-03-10T18:20:34Z

utilities/repostatus.py

+                        wrapped_algorithms.append(algorithm)
+
+# Add a column 'Wrapped' to the DataFrame
+df['Wrapped'] = df.apply(lambda row: 'Yes' if any(algorithm.startswith(row['subfolder'].split('_')[0]) for algorithm in wrapped_algorithms) else 'No', axis=1)


This as well, see above.

etpeterson · 2024-03-10T18:21:35Z

utilities/repostatus.py

+html_string = df_selected.to_html(index=False)
+
+# Save the HTML to a file
+with open(os.path.join(REPO_DIR, 'combined_report.html'), 'w') as f:


I think os.path.join(REPO_DIR, website, 'combined_report.html')

…modifications

…difications

etpeterson · 2024-03-16T01:30:08Z

This is looking good, I think you're almost there. After these small changes I'll send it around to the contributors for a final sanity check.

etpeterson · 2024-04-09T14:04:12Z

Just checking on this, are these changes still in the works?

abhicodes369 · 2024-04-10T05:17:50Z

it looks like this.
Are there any modifications required?

etpeterson · 2024-04-10T05:34:12Z

it looks like this.
Are there any modifications required?

I had 3 comments that look like they haven't been addressed yet. Small but useful like choosing empty rather than no.

abhicodes369 · 2024-04-10T14:30:20Z

i am sorry but i couldnt find these comments could you please give the link to comments that state necessary changes

etpeterson

Sorry! It was my fault, I didn't finalize the review.

etpeterson · 2024-03-16T01:20:15Z

doc/code_contributions_record.csv

-IVIM,Fitting,Segmented NLLS fitting,Specifically tailored algorithm for NLLS segmented fitting,OJ_GU,TF2.4_IVIM-MRI_CodeCollection/src/original/OJ_GU/,Oscar Jalnefjord,University of Gothenburg,seg,https://doi.org/10.1007/s10334-018-0697-5,tbd,
-IVIM,Fitting,Linear fit,Linear fit for D and D* and f. Intended to be extremely fast but not always accurate,ETP_SRI,TF2.4_IVIM-MRI_CodeCollection/src/original/ETP_SRI/LinearFitting.py,Eric Peterson,SRI International,,,tbd,
+Technique,Category,Subcategory,notes,subfolder,Link to source code,Authors,Institution,function/module,DOI,Tester,test status,wrapped
+IVIM,Fitting,LSQ fitting,,OGC_AmsterdamUMC,TF2.4_IVIM-MRI_CodeCollection/src/original/OGC_AmsterdamUMC/,Oliver Gurney-Champion,Amsterdam UMC,fit_least_squares/fit_least_squares_array,,tbd,,no


Rather than no probably just blank is easier to handle later on.

etpeterson · 2024-03-16T01:22:46Z

website/combined_report.html

@@ -5,142 +5,142 @@
      <th>Subfolder</th>
      <th>Contributors</th>
      <th>Tested</th>
-      <th>Wrapped</th>
+      <th>wrapped</th>


website/combined_report.html

etpeterson · 2024-04-10T16:18:02Z

i am sorry but i couldnt find these comments could you please give the link to comments that state necessary changes

Apologies, I didn't finish the review so my comments didn't show for you.

etpeterson · 2024-04-16T04:08:43Z

Looks like a merge conflict in requirements.txt.

etpeterson

Looks good, just those changes then I'll run it locally as a final test.

utilities/repostatus.py

…hicodes369/TF2.4_IVIM-MRI_CodeCollection into created_repostatus/statistics

report on repo

8bd36ed

etpeterson requested changes Mar 8, 2024

View reviewed changes

etpeterson reviewed Mar 9, 2024

View reviewed changes

abhicodes369 added 3 commits March 10, 2024 22:04

created wrapper column

6d210a0

Delete local.env.example

58a423e

Delete .gitignore

6c1b907

Adding recovered .gitignore file

321fa55

etpeterson requested changes Mar 10, 2024

View reviewed changes

abhicodes369 added 3 commits March 15, 2024 23:15

added a new column in the csv file . please tell me if there are any …

cf05312

…modifications

added a new column in the csv file please tell me if there are any mo…

baf5d4a

…difications

added a new column to csv and modified python file

d7df0ca

etpeterson requested changes Apr 10, 2024

View reviewed changes

Refactor column names in repostatus.py and combined_report.html

992147d

etpeterson requested changes Apr 16, 2024

View reviewed changes

utilities/repostatus.py Show resolved Hide resolved

abhicodes369 added 4 commits April 16, 2024 10:53

Add zenodo-get, sphinx, and sphinx_rtd_theme to requirements.txt

6ab39b5

Merge branch 'main' into created_repostatus/statistics

06167e7

wrap the html into a function

4a148b8

Merge branch 'created_repostatus/statistics' of https://github.com/ab…

e17a3e7

…hicodes369/TF2.4_IVIM-MRI_CodeCollection into created_repostatus/statistics

etpeterson approved these changes Apr 17, 2024

View reviewed changes

etpeterson merged commit a6b225d into OSIPI:main Apr 17, 2024
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

report on repo #56

report on repo #56

abhicodes369 commented Mar 8, 2024 •

edited

Loading

etpeterson Mar 8, 2024

etpeterson Mar 8, 2024

etpeterson Mar 8, 2024

etpeterson Mar 8, 2024

etpeterson Mar 8, 2024

etpeterson Mar 9, 2024

abhicodes369 commented Mar 10, 2024

etpeterson commented Mar 10, 2024

etpeterson left a comment

etpeterson Mar 10, 2024

abhicodes369 Mar 11, 2024

etpeterson Mar 11, 2024

etpeterson Mar 10, 2024

abhicodes369 Mar 11, 2024

etpeterson Mar 12, 2024

etpeterson Mar 10, 2024

etpeterson Mar 10, 2024

etpeterson Mar 10, 2024

etpeterson commented Mar 16, 2024

etpeterson commented Apr 9, 2024

abhicodes369 commented Apr 10, 2024

etpeterson commented Apr 10, 2024

abhicodes369 commented Apr 10, 2024

etpeterson left a comment

etpeterson Mar 16, 2024

etpeterson Mar 16, 2024

etpeterson commented Apr 10, 2024

etpeterson commented Apr 16, 2024

etpeterson left a comment

report on repo #56

report on repo #56

Conversation

abhicodes369 commented Mar 8, 2024 • edited Loading

Describe the changes you have made in this PR

Link this PR to an issue [optional]

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abhicodes369 commented Mar 10, 2024

etpeterson commented Mar 10, 2024

etpeterson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpeterson commented Mar 16, 2024

etpeterson commented Apr 9, 2024

abhicodes369 commented Apr 10, 2024

etpeterson commented Apr 10, 2024

abhicodes369 commented Apr 10, 2024

etpeterson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpeterson commented Apr 10, 2024

etpeterson commented Apr 16, 2024

etpeterson left a comment

Choose a reason for hiding this comment

abhicodes369 commented Mar 8, 2024 •

edited

Loading