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

Required parameters for TensorMap #241

Open
ndiamant opened this issue Apr 27, 2020 · 2 comments
Open

Required parameters for TensorMap #241

ndiamant opened this issue Apr 27, 2020 · 2 comments
Labels
enhancement New feature or request

Comments

@ndiamant
Copy link
Contributor

What
Make shape required for TensorMap and check the other parameters if they should be required.

Why
You should get an error making a TensorMap without a shape, because it can't be used in a model. Fail early!

How
Remove the default of None for shape. See if the recipes still run using the smoke tests if other defaults are removed.

Acceptance Criteria
Less Optional parameters.

@ndiamant ndiamant added the enhancement New feature or request label Apr 27, 2020
@lucidtronix
Copy link
Collaborator

Many TensorMaps make use of the shape inference from the channel_map in the TensorMap constructor. I agree we should fail early if that inference fails, but I don't think we need to make shape required.

@lucidtronix
Copy link
Collaborator

Also, I'd vote for removing the Optional type hint altogether. If params have default values they are optional.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants