Skip to content

fixed measurement path bug#1464

Closed
Yantom1 wants to merge 1 commit intohuggingface:mainfrom
HabanaAI:auto-pr-8b2aa20
Closed

fixed measurement path bug#1464
Yantom1 wants to merge 1 commit intohuggingface:mainfrom
HabanaAI:auto-pr-8b2aa20

Conversation

@Yantom1
Copy link
Copy Markdown
Contributor

@Yantom1 Yantom1 commented Oct 30, 2024

What does this PR do?

Fixes # (issue)

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?

@Yantom1 Yantom1 requested a review from regisss as a code owner October 30, 2024 08:47
@libinta libinta changed the title [SW-205252][SW-204329] fixed measurement path bug fixed measurement path bug Oct 31, 2024
measurements_paths.append(measurement_path)
measurement_path = find_measurement_path(measurement, measurements_dir_path, groups_size)
if measurement_path is not None:
measurements_paths.append(measurement_path)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

what should we do if the measurement_path is None?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It won't add the path and will not iterate over it in next steps. we shouldn't do anything

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@Yantom1 it seems you didn't change the function find_measurement_path. can you update that on line 9 def find_measurement_path(measurement, measurements_dir_path, scales, group_size): ?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

seems this needs PR1496

@HolyFalafel
Copy link
Copy Markdown
Contributor

@libinta see here:
#1496

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants