Skip to content

Conversation

@Citrus716
Copy link
Contributor

@Citrus716 Citrus716 commented Aug 7, 2020

Opened by Ke Hao Chen

This pr provides a solution for alternating.py (which apparently had a practice problem but no solution in the main branch). It also fixes img_avg.py's solution to actually work. Lastly, it adds a bit more instruction to img_avg.py to clarify what an image (in list form) would look like.

Citrus716 and others added 5 commits August 6, 2020 17:48
Added an example and other changes to improve understanding of the problem.
Added an example and other changes to improve understanding of the problem. Didn't change the actual solution other than removing the use of functions.
Made the characters per line less in practice/alternating
Added my solution to alternating.py. I didn't make this problem. Good problem though.
Copy link
Contributor

@srikar-eranky srikar-eranky left a comment

Choose a reason for hiding this comment

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

For alternating.py, you might want to convert the alternating numbers into a list, or print them all out on the same line, just to make the output look a bit cleaner. Other than that, I have no issues with these problems or solutions.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Lintly has detected code quality issues in this pull request.

phrdang
phrdang previously requested changes Aug 7, 2020
Copy link
Member

@phrdang phrdang left a comment

Choose a reason for hiding this comment

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

@abhatia1205 Please see my comments for what needs to be changed. Overall, I think adding more comments would be helpful and also please see the error I received (does it work on your computer? Maybe I'm doing something wrong...)

@phrdang phrdang dismissed github-actions[bot]’s stale review August 7, 2020 20:22

Outdated, Black fixed it already

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Lintly has detected code quality issues in this pull request.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

No linting violations have been found in this PR.

Adding the comment noqa: [error code] ignores only that line for flake8.
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

No linting violations have been found in this PR.

Lintly only supports PRs
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

No linting violations have been found in this PR.

Citrus716 and others added 2 commits August 11, 2020 15:29
I read that PIL is dead and hasn't been updated since 2009. To get "PIL" you must pip install pillow because that's the new name. However, when you import, the code is import PIL in order to allow for backward compatibility. Anyways, I made the instructions to install the libraries.
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Lintly has detected code quality issues in this pull request.

Copy pasted from my instructions I added in practice.
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

No linting violations have been found in this PR.

Swapped i and j descriptions and added more. The i represents top to bottom(height) while the j represents left to right(width).
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Lintly has detected code quality issues in this pull request.

@BenVN123
Copy link
Collaborator

BenVN123 commented Aug 9, 2021

This PR is really old. Is it still valid?

@chrehall68
Copy link
Contributor

Yeah so I just looked at it and it's still valid since main's img_avg solution doesn't work

@chrehall68
Copy link
Contributor

Unfortunately, neither does this branch's img_avg solution

@chrehall68 chrehall68 requested a review from phrdang August 9, 2021 17:26
@chrehall68 chrehall68 dismissed github-actions[bot]’s stale review August 9, 2021 17:32

obsolete 'changes requested' from linting which now passes

@chrehall68 chrehall68 requested a review from BenVN123 August 9, 2021 17:33
@chrehall68
Copy link
Contributor

Alright I fixed it. Now, it works. This is much better compared with the dysfunctional version that we have on main branch.

chrehall68 and others added 6 commits August 9, 2021 10:40
The bug was that the averages were slightly off because I forgot to set y_relative_indexes to an empty list (which I now did, which is why it's now fixed)
@chrehall68 chrehall68 dismissed phrdang’s stale review August 9, 2021 18:48

dismissing obsolete review

@chrehall68 chrehall68 merged commit 71adc1b into master Aug 9, 2021
@chrehall68 chrehall68 deleted the kc_req_changes branch August 9, 2021 18:48
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.

7 participants