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

Error when trying to encode scalar Enum arrays #922

Closed
bonjourmauko opened this issue Jan 1, 2020 · 3 comments
Closed

Error when trying to encode scalar Enum arrays #922

bonjourmauko opened this issue Jan 1, 2020 · 3 comments
Labels
kind:fix Bugs are defects and failure demand. level:starter Suited for beginners and new contributors

Comments

@bonjourmauko
Copy link
Member

bonjourmauko commented Jan 1, 2020

The following code fails for scalar Enum arrays (without dimension):

if len(array) > 0 and cls.__name__ is array[0].__class__.__name__:

A real-life example of this bug:

https://github.com/openfisca/openfisca-france/blob/7918eec5be4c2cdeb65da9fb023cfbfcce537156/openfisca_france/model/prestations/logement_social.py#L71-L81

You can reproduce the issue with the following code:

from numpy import array, atleast_1d

from openfisca_core.indexed_enums import Enum

class Greetings(Enum):
    hola = "Hola"
    chao = "Chao"

greeting = array(Greetings.hola)
greeting.size # 1
len(greeting) # TypeError: len() of unsized object
atleast_1d(greeting).size # 1
len(atleast_1d(greeting)) # 1

What's the expected behaviour here?

  1. We're expecting an array with at least one dimension.
  2. We should make sure we cast or take into account scalar arrays.

cc @Morendil @fpagnoux

@bonjourmauko bonjourmauko added the kind:fix Bugs are defects and failure demand. label Jan 1, 2020
@bonjourmauko bonjourmauko added the level:starter Suited for beginners and new contributors label Jan 2, 2020
@Morendil
Copy link
Contributor

Morendil commented Jan 2, 2020

Are you sure? The code in France you are linking to was exercised by France tests and did not seem to be affected by this bug, prior to Core 34.5.4.

(See this comment)

@bonjourmauko
Copy link
Member Author

Not really sure, no : as there it no signature in the original code, I don't know what is the expected behaviour.

So, #923 adds tests assuming we wanted to handle scalar vectors as well.

@bonjourmauko
Copy link
Member Author

bonjourmauko commented Jan 2, 2020

So as discussed in openfisca/openfisca-france#1395, it seems the current behaviour is the expected behaviour: expecting a vector.

I'm then :

Thanks !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:fix Bugs are defects and failure demand. level:starter Suited for beginners and new contributors
Projects
None yet
Development

No branches or pull requests

2 participants