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

Ignore empty description columns #52

Merged
merged 1 commit into from
Aug 7, 2015
Merged

Ignore empty description columns #52

merged 1 commit into from
Aug 7, 2015

Conversation

vzctl
Copy link
Collaborator

@vzctl vzctl commented Aug 7, 2015

Reckon extracts a list of tokens not from a CSV row itself, but from a generated ledger description line using tr.downcase.split(/[\s\-]/) which also includes ; in the list of the tokens.

If reckon detects two description columns and one of them is empty it generates unnecessary ; in the ledger row and it breaks tokenization.

This PR does not fix the root cause of the problem but it fixes the immediate issue in #51 and adds some cosmetic benefits.

To fix the root cause probably need to improve token extraction and extend token list by splitting the description by non-word symbols...

@vzctl vzctl mentioned this pull request Aug 7, 2015
@cantino
Copy link
Owner

cantino commented Aug 7, 2015

Looks good, thanks for fixing!

vzctl added a commit that referenced this pull request Aug 7, 2015
Ignore empty description columns
@vzctl vzctl merged commit 7c1962a into cantino:master Aug 7, 2015
@vzctl
Copy link
Collaborator Author

vzctl commented Aug 7, 2015

@cantino, do you want to push gem releases manually? we can configure travis to do it automatically: http://docs.travis-ci.com/user/deployment/rubygems/

@cantino
Copy link
Owner

cantino commented Aug 7, 2015

I can push it tonight. And that's a great suggestion.

@cantino
Copy link
Owner

cantino commented Aug 8, 2015

v0.4.2 released!

@briviere
Copy link

Well I tried the latest version, just to let you know version has not been updated.

Anyhow I tried the latest using the data from #51, unfortunately it is still not working.

Thanks

Brian

@cantino
Copy link
Owner

cantino commented Aug 15, 2015

What do you mean that it hasn't been updated? v0.4.2 is here.

@briviere
Copy link

I ment the VERSION in app.rb App class was not updated:

--- a/lib/reckon/app.rb
+++ b/lib/reckon/app.rb
@@ -4,7 +4,7 @@ require 'yaml'

module Reckon
class App

  • VERSION = "Reckon 0.3.10"
  • VERSION = "Reckon 0.4.2"
    attr_accessor :options, :accounts, :tokens, :seen, :csv_parser

def initialize(options = {})

@cantino
Copy link
Owner

cantino commented Aug 16, 2015

Oh! You're completely correct, thank you for noticing that. I just pushed v0.4.3 to fix this.

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