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

Type checking in TableModel #2238

Merged
merged 2 commits into from
Jun 26, 2024
Merged

Type checking in TableModel #2238

merged 2 commits into from
Jun 26, 2024

Conversation

samtygier-stfc
Copy link
Collaborator

@samtygier-stfc samtygier-stfc commented Jun 26, 2024

Issue

Some type checking for #2213

Description

This adds some type annotations to TableModel. This is slightly awkward because the data rows are a heterogenous list, and there is not way to strictly type its elements. We can't switch to a tuple, because they need to be mutable (maybe we can. but it is more code change).

So add ElementType and RowType, which limit the types, but don't understand the positions. and add an overloaded get_element() that fixes the type based on the literal value of the column argument.

Also some changes in the view to access things though get_element() and set_element()

Acceptance Criteria

Existing tests should all still pass
Play around with adding, removing and renaming ROIs

Documentation

Not needed

@coveralls
Copy link

Coverage Status

coverage: 73.142% (-0.01%) from 73.156%
when pulling 1588106 on 2213-type-checking-2
into d407243 on main.

@samtygier-stfc samtygier-stfc marked this pull request as ready for review June 26, 2024 10:44
@JackEAllen JackEAllen self-requested a review June 26, 2024 10:48
@coveralls
Copy link

Coverage Status

coverage: 73.137% (-0.02%) from 73.156%
when pulling b9cc693 on 2213-type-checking-2
into 38eeb61 on main.

Copy link
Collaborator

@JackEAllen JackEAllen left a comment

Choose a reason for hiding this comment

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

Nice use of overloading

@JackEAllen JackEAllen added this pull request to the merge queue Jun 26, 2024
Merged via the queue into main with commit f13ad18 Jun 26, 2024
8 checks passed
@JackEAllen JackEAllen deleted the 2213-type-checking-2 branch June 26, 2024 12:40
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.

3 participants