Skip to content

Conversation

@alexrudd2
Copy link
Collaborator

See #2084.
This is mostly a demonstration of how TypedDict could be used.

However, it made me question whether we should remove the Label abstraction.
What is the purpose for cases where the label is identical to the key? e.g. Label.value == 'value'.

Would it not be easier to only use it when they differ e.g. Label.co_size == 'co size'?

Could the labels be generated programmatically? (It looks like replacing a space with an underscore and remove type_ prefix would work in all cases).

@alexrudd2 alexrudd2 changed the title Use a TypedDict Use a TypedDict for simulator type hints Mar 5, 2024
@alexrudd2 alexrudd2 force-pushed the simulator-types-typeddict branch from 707d2ce to fbf9f73 Compare March 5, 2024 17:52
@janiversen
Copy link
Collaborator

The reason for

Label.value = 'value'

is so the program can use Label.value, while the configuration json uses 'value', that is needed unless you come up with an alternative.

@janiversen
Copy link
Collaborator

The label can possible be done programmatically....however Label is an enum, and should stay that way.

Apart from that, the whole simulator configuration is way to complex, and it is on my list to make a new version.

Base automatically changed from simulator-types to dev March 5, 2024 20:15
@janiversen
Copy link
Collaborator

But as usual I am open to suggestions....

@janiversen
Copy link
Collaborator

Just request review when you think it is ready.

@alexrudd2
Copy link
Collaborator Author

I just tried the simulator for the first time today, so hopefully I have a better understanding soon.

@github-actions
Copy link

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions
Copy link

This PR was closed because it has been stalled for 10 days with no activity.

@github-actions github-actions bot closed this Apr 30, 2024
@janiversen janiversen deleted the simulator-types-typeddict branch June 1, 2024 17:48
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