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

All function to deal with np.nan #86

Open
jbesomi opened this issue Jul 14, 2020 · 1 comment
Open

All function to deal with np.nan #86

jbesomi opened this issue Jul 14, 2020 · 1 comment
Assignees
Labels
enhancement New feature or request testing

Comments

@jbesomi
Copy link
Owner

jbesomi commented Jul 14, 2020

(Edit)

As it is the case with Pandas, every texthero function should deal with not assigned (NA) values.

The rule of thumb should be the following:

If the given input Pandas Series s has some NA at indexes X, the same output Pandas Series should have the same nan at indexes X. ("NA rule")

This task consists in:

  1. Add a test_na.py file that checks for every "texthero" function that the NA rule is respected
  2. Change any functions that fail the NA rule (see Implement & test handling of NaNs in preprocessing with handle_nans decorator #130 ) for some input

This task has appeared to be more complex than initially thought.

Linked PR: #86, #130 and #123

@mk2510
Copy link
Collaborator

mk2510 commented Jul 16, 2020

i will start working on this issue together with @henrifroese

henrifroese added a commit to SummerOfCode-NoHate/texthero that referenced this issue Jul 17, 2020
Every function in the library now handles NaNs correctly.

Implemented through decorator @handle_nans in new file _helper.py.

Tests added in test_nan.py

As we went through the whole library anyways, argument "input" was renamed to "s" in some functions to be in line with the others.

Co-authored-by: Maximilian Krahn <maximilian.krahn@icloud.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request testing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants