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

Df 131 #39

Merged
merged 36 commits into from
Feb 4, 2025
Merged

Df 131 #39

merged 36 commits into from
Feb 4, 2025

Conversation

Si2-Aung
Copy link
Collaborator

@Si2-Aung Si2-Aung commented Jan 30, 2025

DQL-<NUM>: <TITLE>

<DESCRIPTION>

✅ Pre-Merge Checklist

All boxes must be checked before the Pull Request is merged. This checklist has to be completed by the reviewer.

  • The CI pipeline has passed (this is enforced by GitHub)
  • Code Review
    • The code has been manually inspected by someone who did not implement the feature
  • The PR implements what is described in the JIRA-Issue
  • The new feature has been tested by the reviewer on his local machine
  • A deployment to the dev environment has succeeded. If possible, ensure that the feature can be observed on the server.
  • If required, any documentation is updated accordingly

💾 Merge Checklist

  • The branch is merged to main using the "Squash and Merge" strategy following the commit message naming convention DQL-<NUM>: <TITLE>
  • The branch is deleted after the PR is merged

Copy link
Collaborator

@TilmanNiem TilmanNiem left a comment

Choose a reason for hiding this comment

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

Beim Testing ist mir aufgefallen, dass sehr viele Fehler auftreten bei der string to float conversion mit verschiedenen numerischen Werten, das können wir uns vielleicht nochmal anschauen. Außerdem sind sehr viele (unnötige) Kommentare drin und #noqa Statements, die sollten reduziert/so gut es geht entfernt werden.

Copy link
Collaborator

@jonkra20 jonkra20 left a comment

Choose a reason for hiding this comment

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

Ich habe dir an die entsprechenden files Kommentare geschrieben

tests/review/test_dataset_reviewer.py Outdated Show resolved Hide resolved
tests/review/test_numeric_value_generator.py Outdated Show resolved Hide resolved
src/dataland_qa_lab/review/yes_no_value_generator.py Outdated Show resolved Hide resolved
src/dataland_qa_lab/review/generate_gpt_request.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@jonkra20 jonkra20 left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Collaborator

@TilmanNiem TilmanNiem left a comment

Choose a reason for hiding this comment

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

Passt

@Si2-Aung Si2-Aung merged commit 62018e4 into main Feb 4, 2025
2 checks passed
@Si2-Aung Si2-Aung deleted the df-131 branch February 4, 2025 21:19
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.

5 participants