fix fpmax issue (#570) with fptrees that contain no nodes

[harenbergsd]

Description

Fpmax had a bug with empty (other than root node) fptrees because it was basing the support calculation from nodes in the tree. I have changed it so that the root node in the tree contains the total support (ie the support of the conditional items that the tree is based on). Since every tree will have at least the root node this fixes the issue.

Related issues or pull requests

Fixes #570

Pull Request Checklist

• Added a note about the modification or contribution to the ./docs/sources/CHANGELOG.md file (if applicable)
• Added appropriate unit test functions in the ./mlxtend/*/tests directories (if applicable)
• Modify documentation in the corresponding Jupyter Notebook under mlxtend/docs/sources/ (if applicable)
• Ran PYTHONPATH='.' pytest ./mlxtend -sv and make sure that all unit tests pass (for small modifications, it might be sufficient to only run the specific test file, e.g., PYTHONPATH='.' pytest ./mlxtend/classifier/tests/test_stacking_cv_classifier.py -sv)
• Checked for style issues by running flake8 ./mlxtend

[coveralls]

Coverage increased (+0.1%) to 92.252% when pulling d42ac1b on harenbergsd:fpmax-bug-570 into ac0f0c1 on rasbt:master.

[harenbergsd]

I am going to update and expand unit tests a bit, so will have one or more commits coming.

[rasbt]

Thanks for the update and addressing this issue!

I am going to update and expand unit tests a bit, so will have one or more commits coming.

Sounds good! I will wait with merging then.

[harenbergsd]

I added some unit tests and refactored a bit. I moved FPMax specific code to test_fpmax. Since apriori and fpgrowth share the same code, I left the common tests in test_fpbase.

Ultimately, we'd like to run the exact same tests for fpgrowth, apriori, and apriori_low_mem, but I am not sure how to get a cleaner, more concise way to to do that...

[rasbt]

I just looked over the refactoring of the unit tests, and I think that's great and serves the purpose well enough -- no need to make it even more elegant I guess :). Looks like this should also be good to merge then? Thanks again for putting all the effort into it!

[harenbergsd]

Yep sounds good

…

On Mon, Aug 5, 2019, 9:08 AM Sebastian Raschka ***@***.***> wrote: I just looked over the refactoring of the unit tests, and I think that's great and serves the purpose well enough -- no need to make it even more elegant I guess :). Looks like this should also be good to merge then? Thanks again for putting all the effort into it! — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#573?email_source=notifications&email_token=ACAPQ233WLML4IWXKQ2G7ADQDAQ6BA5CNFSM4IJDVEG2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3RYJQI#issuecomment-518227137>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACAPQ23Y3HYGG7E5T2I66JLQDAQ6BANCNFSM4IJDVEGQ> .