ymoch / apyori

A simple implementation of Apriori algorithm by Python.
MIT License
243 stars 93 forks source link

Fix load_transactions() newline bug #34

Closed ZaxR closed 4 years ago

ZaxR commented 6 years ago

load_transactions() currently iterates through characters in the input_file string. Example of the issue can be found here: https://stackoverflow.com/questions/43348498/using-the-load-transactions-function-in-the-apyori-package-in-python/46651675#46651675 . Explicitly defining newline fixes the issue. Also utilized context management.

ymoch commented 6 years ago

Thanks for your request, but your changes doesn't satisfy the tests because load_transaction() can't take file-like objects... In my opinion, load_transaction() should be able to takes a file-like object as the csv.reader() do, even if it can also take a file path.

For the case in the Stack Overflow, opening file externally and giving the file object to load_transaction() can solve that problem as below.

with open(file_path) as file_obj:
    transactions = load_transaction(file_obj)
ZaxR commented 6 years ago

Thanks for the fast response - that makes sense. For users less familiar with the duck typing nature of python, would it make sense to offer a load_transactions_from_file() method that's just a wrapper for load_transactions()? Alternatively, adding your snippet above to the readme would be awesome, I think. Thank you for the package and support!

P.S. I updated my response to the SO question to include your answer.

ymoch commented 6 years ago

I'm very sorry that I've forgot to reply... and thank you for your SO answer! Though your offer to implement load_transactions_from_file() makes sence, I have another idea that load_transaction() open and load a file when given a PathLike object (strings or bytes too). Does it help?