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 earlier stage, or convert types #106

Closed
veenstrajelmer opened this issue Oct 30, 2023 · 2 comments
Closed

Type checking in earlier stage, or convert types #106

veenstrajelmer opened this issue Oct 30, 2023 · 2 comments

Comments

@veenstrajelmer
Copy link
Collaborator

veenstrajelmer commented Oct 30, 2023

Describe the bug
When passing a node_x array of a wrong type to meshkernel.Mesh2d there is no error raised. However, when passing the resulting Mesh2d instance to meshkernel.MeshKernel.mesh2d_set, there is type checking. Also, I would say int/float32 could be converted to float64 by meshkernel.

To Reproduce

import meshkernel
import numpy as np

node_x = np.array([1,2,3])
node_y = np.array([1,2,1])
edge_nodes = np.array([[0,1],[1,2],[2,0]])

mesh2d = meshkernel.Mesh2d(
    node_x=node_x.astype(np.float32), # deliberately using false type (this can come from an array like this)
    node_y=node_y.astype(np.float64),
    edge_nodes=edge_nodes.ravel().astype(np.int32),
)

mk = meshkernel.MeshKernel()

mk.mesh2d_set(mesh2d) # type checking only happens upon mesh2d_set, not upon Mesh2d.__init__. Also, these types can be converted into each other

mk.mesh2d_set(mesh2d) raises TypeError: incompatible types, c_long_Array_3 instance instead of LP_c_double instance

Expected behavior
type checking in an earlier stage (upon Mesh2d init). Also, accepting types that can be converted, or catching them in/before mesh2d_set

Version info (please complete the following information):

  • OS: Windows
  • Version 3.0.0
@ahmad-el-sayed
Copy link
Contributor

ahmad-el-sayed commented Nov 28, 2023

This is not a bug. The wrong data type is used. mesh2d_set fails because the data type is enforced in class Mesh via type_enforced decorator. Types are not enforced elsewhere including in class Mesh2D.

A decision will be made if type enforcement is to be retained. The data type hints and the doc should suffice.

@ahmad-el-sayed ahmad-el-sayed added invalid This doesn't seem right and removed bug Something isn't working invalid This doesn't seem right labels Nov 28, 2023
@ahmad-el-sayed
Copy link
Contributor

It was decided not to enforce types at all. Type enforcement was removed in #122.

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

No branches or pull requests

2 participants