-
Notifications
You must be signed in to change notification settings - Fork 15
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
fix imprecise resource type-hinting #534
fix imprecise resource type-hinting #534
Conversation
10af708
to
6056147
Compare
if len(set(r.component for r in initialized_resources)) != 1: | ||
raise ResourceError("All initialized resources must have the same component.") | ||
|
||
if len(set(r.resource_type for r in initialized_resources)) != 1: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean you can have duplicates in initialized_resources? That seems weird to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This case will only occur in practice, when a component is creating multiple columns. Then there will be multiple Column resources in the group. This is throwing an error if the resources are not all of the same type.
return False | ||
|
||
|
||
class NullResource(Resource): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I have asked this before but what is the point of this class? I am guessing for warnings/errors later on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because a component may have an on_initialize_simulants
method that has required resources, even if that component doesn't create any columns. This is the resource that corresponds to that case.
tests/conftest.py
Outdated
@@ -31,6 +32,11 @@ def pytest_collection_modifyitems(config, items): | |||
item.add_marker(skip_slow) | |||
|
|||
|
|||
@pytest.fixture(scope="session") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this will save the output diagnostic file to this directory but it doesn't look like you needed to ignore that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this comment
Fix imprecise resource type-hinting
Description
Changes and notes
This was introduced as a result of merge conflicts
Testing
All builds pass