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

Feature / Columnar Data helper function #830

Closed
wants to merge 3 commits into from

Conversation

nitbharambe
Copy link
Member

Implements #786

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>
src/power_grid_model/_core/power_grid_meta.py Outdated Show resolved Hide resolved
src/power_grid_model/_core/power_grid_meta.py Outdated Show resolved Hide resolved
src/power_grid_model/_core/power_grid_meta.py Show resolved Hide resolved
tests/unit/test_meta_data.py Outdated Show resolved Hide resolved
tests/unit/test_meta_data.py Outdated Show resolved Hide resolved
src/power_grid_model/_core/power_grid_meta.py Outdated Show resolved Hide resolved
Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>
Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>
@nitbharambe
Copy link
Member Author

Issue with type hinting:

initialize_array now gives out row based data, hence the type is np.ndarray.
When we assign like how it is done in example, node["id"] = [1,2,3], we expect to use assignment of attribute within the array.

If we are to implement this feature, the return type is np.ndarray | dict[str, np.ndarray].
If user tries to do the same assignment in case of the columnar data, ie. when node is dict[str, np.ndarray], the [] operator refers to dictionary and thus a numpy array gets replaced with a list. Hence, this correctly gives out error with mypy.

Thus this helper function might end up confusing the user. If the user does the assigning / creation etc themselves via power_grid_meta_data, such operations might be more clear to them.

@nitbharambe
Copy link
Member Author

Closed because of this reason. attribute_dtype and Attribute_empty implemented in #786

Issue with type hinting:

initialize_array now gives out row based data, hence the type is np.ndarray. When we assign like how it is done in example, node["id"] = [1,2,3], we expect to use assignment of attribute within the array.

If we are to implement this feature, the return type is np.ndarray | dict[str, np.ndarray]. If user tries to do the same assignment in case of the columnar data, ie. when node is dict[str, np.ndarray], the [] operator refers to dictionary and thus a numpy array gets replaced with a list. Hence, this correctly gives out error with mypy.

Thus this helper function might end up confusing the user. If the user does the assigning / creation etc themselves via power_grid_meta_data, such operations might be more clear to them.

@figueroa1395
Copy link
Contributor

@nitbharambe Looks like you have to manually delete the branch due to unmerged commits. Don't forget :)

@nitbharambe nitbharambe deleted the feature/columnar-data-helper-fn branch November 18, 2024 08:04
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