Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Solve 2021 day 01 #1

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Solve 2021 day 01 #1

wants to merge 14 commits into from

Conversation

katzuv
Copy link
Owner

@katzuv katzuv commented Dec 1, 2021

No description provided.

@katzuv katzuv self-assigned this Dec 1, 2021
@katzuv katzuv requested a review from YoniKF December 2, 2021 10:41
Copy link
Collaborator

@YoniKF YoniKF left a comment

Choose a reason for hiding this comment

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

Reviewed first problem.

from pathlib import Path
from typing import Iterator

INPUT_FILE_PATH = Path('..', 'inputs', '1.txt')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use __file__ instead of a relative path.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Not sure how __file__ fits in here, could you explain?

return sum(couple[1] > couple[0] for couple in couples)


def main():
Copy link
Collaborator

Choose a reason for hiding this comment

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

Write the I/O logic once, and for each problem implement only the pure logic: A function receiving the input as a string and returning the output as a string.

Then you can maybe think about an automation for running all the problems and uploading the results.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I'm not sure it will be very helpful, getting the input from the file is one line. Or did you mean something else?

Copy link
Collaborator

Choose a reason for hiding this comment

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

And the main function and defining the file's locations and printing the result...

2021/d01/p1.py Outdated Show resolved Hide resolved
2021/d01/p1.py Outdated Show resolved Hide resolved
2021/d01/p1.py Outdated
:param measurements: measurements to parse
:return: count of the number of times a depth measurement increases
"""
couples = zip(measurements[:-1], measurements[1:])
Copy link
Collaborator

Choose a reason for hiding this comment

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

pairs is a better term.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Thanks, after I finished my solution I found out there's a new pairwise function in itertools. Figured out if I was going to change something, better to use the new function already.

Copy link
Collaborator

@YoniKF YoniKF left a comment

Choose a reason for hiding this comment

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

Finished review.

2021/d01/p1.py Outdated Show resolved Hide resolved
2021/d01/p2.py Outdated
_DEFAULT_WINDOW_SIZE = 3


def get_measurements_windows(measurements: Iterator[int], window_size: int = _DEFAULT_WINDOW_SIZE) -> list[tuple[int]]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Think how you can use zip for the general window size. Hint: argument list unpacking.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Was 5675ce9 what you expected?

from pathlib import Path
from typing import Iterator

INPUT_FILE_PATH = Path('..', 'inputs', '1.txt')
Copy link
Owner Author

Choose a reason for hiding this comment

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

Not sure how __file__ fits in here, could you explain?

2021/d01/p1.py Outdated Show resolved Hide resolved
2021/d01/p1.py Outdated Show resolved Hide resolved
return sum(couple[1] > couple[0] for couple in couples)


def main():
Copy link
Owner Author

Choose a reason for hiding this comment

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

I'm not sure it will be very helpful, getting the input from the file is one line. Or did you mean something else?

2021/d01/p1.py Outdated
:param measurements: measurements to parse
:return: count of the number of times a depth measurement increases
"""
couples = zip(measurements[:-1], measurements[1:])
Copy link
Owner Author

Choose a reason for hiding this comment

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

Thanks, after I finished my solution I found out there's a new pairwise function in itertools. Figured out if I was going to change something, better to use the new function already.

2021/d01/p1.py Outdated
INPUT_FILE_PATH = Path('..', 'inputs', '1.txt')


def get_measurements_from_input(input_text: str) -> list[int]:
Copy link
Owner Author

Choose a reason for hiding this comment

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

Suggested change
def get_measurements_from_input(input_text: str) -> list[int]:
def get_measurements_from_input(input_text: str) -> list[int]:t

2021/d01/p1.py Outdated Show resolved Hide resolved
2021/d01/p2.py Outdated
_DEFAULT_WINDOW_SIZE = 3


def get_measurements_windows(measurements: Iterator[int], window_size: int = _DEFAULT_WINDOW_SIZE) -> list[tuple[int]]:
Copy link
Owner Author

Choose a reason for hiding this comment

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

Was 5675ce9 what you expected?

@katzuv katzuv requested a review from YoniKF December 2, 2021 21:27
@katzuv katzuv added the solution Puzzle solution label Dec 9, 2022
@YoniKF YoniKF removed their request for review February 25, 2024 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
solution Puzzle solution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants