Skip to content

Conversation

@baraldian
Copy link
Contributor

fix: refactor fillna and mobility_filter lambda functions with functions, fillna_safe also applies fillna from pandas to solve multiple nan encodings; creating directory for cached data when not available.

@jenno-verdonck
Copy link
Contributor

Could the proposed fix for multiple nan encoding also help in the generate_categories function? Currently the code uses a placeholder value (e.g. -99999999999999.0) because using a nan value in a python dictionary doesn't really work as each nan is seen as a different key.

@baraldian
Copy link
Contributor Author

Could the proposed fix for multiple nan encoding also help in the generate_categories function? Currently the code uses a placeholder value (e.g. -99999999999999.0) because using a nan value in a python dictionary doesn't really work as each nan is seen as a different key.

I'm sorry, but given that I'm external to the project, I'm fixing only the things that are giving problems to my experiments.

@mrtzh mrtzh self-assigned this May 1, 2024
return load_definitions(root_dir=self._root_dir, year=self._survey_year, horizon=self._horizon,
download=download)

def fillna_safe(x, value=-1):
Copy link
Member

@mrtzh mrtzh May 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you describe what the intended behavior of this function should be?

Note that np.nan_to_num will convert nans to 0.0 unless you set the keyword nan=value. See here.

So, currently, I believe this function will first convert all nans to zero and then pass an array without any nans to pandas.

Am I getting this wrong?

Copy link
Member

@mrtzh mrtzh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there may be an issue with fillna_safe.

@mrtzh
Copy link
Member

mrtzh commented May 1, 2024

See also discussion in #39 about the nan_to_num problem.

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