Skip to content

Conversation

@jokasimr
Copy link
Contributor

@jokasimr jokasimr commented Apr 23, 2025

To select the correct rows from the data frames we must use Datagrid.get_visible_rows() when indexing to get the selected rows. The indices returned by Datgrid.selection refers to the indices in the visible part of the dataframe.

Depends on #148

@jokasimr jokasimr force-pushed the gui-select-reference branch from e3ff207 to fab8776 Compare April 24, 2025 12:01
@github-project-automation github-project-automation bot moved this to In progress in Development Board Apr 24, 2025
@jokasimr jokasimr moved this from In progress to Selected in Development Board Apr 24, 2025
Comment on lines 94 to 95
row_idx = selections[0]['r1']
run = self.runs_table.data.iloc[row_idx]['Run']
run = self.runs_table.get_visible_data().iloc[row_idx]['Run']
Copy link
Member

Choose a reason for hiding this comment

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

Do you use the row_idx anywhere else in this function (and other functions below)?
If you don't, how about making a method named e.g. get_run_number() which would hide all the logic?
It appears that all methods are using the same code?

    def get_run_number(self, selections):
        row_idx = selections[0]['r1']
        return self.runs_table.get_visible_data().iloc[row_idx]['Run']

I don't know if you can even include the selections = self.runs_table.selections in there to reduce duplication further?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

how about making a method named e.g. get_run_number() which would hide all the logic?

If you look at the other cases when we use selections (in the other classes in the module) you see that we don't always just get the first row from the selected region.

We could make a helper function, but I don't think it's that useful. The difference would be that instead of remembering to call grid.get_visible_data() we would have to remember to call helper_function_name(grid) when accessing data from a datagrid.

That said, we could do something like this:

def iloc_for_selected(grid):
    return grid.get_visible_data().iloc

and it would be used like

iloc_for_selected(self.runs_table)[row_idx]['Run']

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tried adding the helper function and I think that was a bit nicer.

@jokasimr jokasimr merged commit 7d41c78 into main Apr 28, 2025
4 checks passed
@github-project-automation github-project-automation bot moved this from Selected to Done in Development Board Apr 28, 2025
@jokasimr jokasimr deleted the gui-select-reference branch April 28, 2025 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants