Skip to content

Conversation

@AzaliaAlisheva
Copy link

Description of changes

@ZolotarevAlexandr ZolotarevAlexandr self-requested a review December 2, 2025 16:13
Copy link
Member

@ZolotarevAlexandr ZolotarevAlexandr left a comment

Choose a reason for hiding this comment

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

Everything's fine, I marked couple of things that should be corrected (nothing too big), but overall everything works


from src.logging_ import logger

# TODO: Rework to work with xlsx instead (better to def parse(dfs: dict[str, pd.DataFrame]) -> dict[str, list[date]])...
Copy link
Member

Choose a reason for hiding this comment

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

You implemented it, so better to remove that todo

days = df.iloc[2::2, :]
# drop first, second and days rows
df = df.drop(df.index[[0, 1, *range(2, len(df), 2)]])
# flatten days
Copy link
Member

Choose a reason for hiding this comment

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

Some comments are a bit excessive (like that one). But it's nice that there are comments in the first place


# TODO: Rework to work with xlsx instead (better to def parse(dfs: dict[str, pd.DataFrame]) -> dict[str, list[date]])...

def process_dataframe(df, entries):
Copy link
Member

Choose a reason for hiding this comment

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

We have type hints for function arguments and return values, add them for that one

"""

# TODO: Use parse_cleaning_html.py instead
logger.warning("Not implemented")
Copy link
Member

Choose a reason for hiding this comment

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

Well, it is now)

# rdate=dates,
# )
# )
spreadsheet_id = "1xXnyinI1sNQ3ZKTPlKlqJKt4685oCz2R2LzlgEUztKs"
Copy link
Member

Choose a reason for hiding this comment

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

Such things better to keep in config file

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.

2 participants