Skip to content

Assignment 2#3

Open
PapaVafa wants to merge 4 commits intomainfrom
Assignment_2
Open

Assignment 2#3
PapaVafa wants to merge 4 commits intomainfrom
Assignment_2

Conversation

@PapaVafa
Copy link
Copy Markdown
Owner

@PapaVafa PapaVafa commented Mar 6, 2025

What changes are you trying to make? (e.g. Adding or removing code, refactoring existing code, adding reports)

Answered the questinos

What did you learn from the changes you have made?

some codes

Was there another approach you were thinking about making? If so, what approach(es) were you thinking of?

N/A

Were there any challenges? If so, what issue(s) did you face? How did you overcome it?

It was challening

How were these changes tested?

yes

A reference to a related issue in your repository (if applicable)

Checklist

  • I can confirm that my changes are working as intended

Copy link
Copy Markdown

@juliagallucci juliagallucci left a comment

Choose a reason for hiding this comment

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

part 1 should use readlines() as suggested by the assignment instructions

@danielrazavi
Copy link
Copy Markdown

Hey! Thanks for your submission. Because you went ahead and decided to use "requests", I see that you had to update the paths to fetch the files from the web. That wasn’t necessary and, more importantly, not the point of the assignment. The files are meant to be accessed locally, so there’s no need for an HTTP request or modifying the paths.

The assignment specifically suggests using readline() or readlines(), which would have been the simpler and more appropriate way to read the file. Please update your work to follow that approach instead. Let me know if you have any questions! 😊

Copy link
Copy Markdown

@danielrazavi danielrazavi left a comment

Choose a reason for hiding this comment

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

Look great! Thank you for fixing the issues. Marking as approved.

" mean_inflammations = patient_summary(file_path, 'mean')\n",
"\n",
" # Use check_zeros to check if any mean inflammation scores are zero\n",
" if check_zeros(mean_inflammations):\n",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I mean check_zeros already gives out a boolean answer, so you could've simplified this if statement here, but that doesn't mean what you did was wrong. Just something to think about. 😄

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