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

Adds LX and REF for transactions #14

Merged
merged 1 commit into from
Sep 28, 2024

Conversation

rposborne
Copy link

@rposborne rposborne commented Sep 27, 2024

This resolves all stdout warnings of
Identifier: LX not handled in transaction loop.
Identifier: REF not handled in transaction loop.

While these fields don't seem super useful no warnings is quite nice.

This also adds a nevada medicaid sample found here https://www.medicaid.nv.gov/Downloads/provider/Sample_835_File.pdf

Specs are here
https://www.cms.gov/medicare/billing/electronicbillingeditrans/downloads/835-flatfile.pdf

Grep for EV, and TJ
AT-5936

@rposborne rposborne requested a review from curtisim0 September 27, 2024 14:34
This resolves all stdout warnings of
Identifier: LX not handled in transaction loop.
Identifier: REF not handled in transaction loop.

While these fields don't seem super useful no warnings is quite nice.

This also adds a nevada medicaid sample found here https://www.medicaid.nv.gov/Downloads/provider/Sample_835_File.pdf

[AT-5936]

Signed-off-by: Russell Osborne <[email protected]>
@rposborne rposborne force-pushed the rpo/add-lx-and-ref-for-transactions branch from f3fe700 to 6e1f542 Compare September 27, 2024 14:35
Comment on lines +135 to +137
@property
def production_date(self) -> datetime.datetime:
return self.date.date
Copy link
Author

Choose a reason for hiding this comment

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

Just a convenience method.


elif identifier in cls.terminating_identifiers:
return transaction, segments, segment

else:
segment = None
message = f'Identifier: {identifier} not handled in transaction loop.'
message = f'Identifier: {identifier} not handled in transaction loop. {segment}'
Copy link
Author

Choose a reason for hiding this comment

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

I found seeing the segment right away VERY useful to go check specs.

Choose a reason for hiding this comment

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

Freaking neeeeeeeeed a better pattern here. But yes, this is gud 👍

segment = split_segment(segment)

self.identifier = segment[0]
self.number = segment[1]
Copy link
Author

Choose a reason for hiding this comment

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

Still feel the need for a base class here.

Choose a reason for hiding this comment

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

fo sho

@@ -12,6 +12,8 @@ python = "^3.9"
pandas = "^2.0.3"
pytest = "^8.1.1"

[tool.poetry.dev-dependencies]
ruff = "*"
Copy link
Author

Choose a reason for hiding this comment

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

Snuck this in.

@@ -4,6 +4,7 @@
import pytest

sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), '..')))
sys.path.append(os.path.join(os.path.dirname(__file__), 'utils'))
Copy link
Author

Choose a reason for hiding this comment

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

Open to different approaches here, but I wanted to separate the test helpers from the tests.

Comment on lines +38 to +44
def get_claim_by_control_number(transaction_set, claim_id):
for transaction in transaction_set.transactions:
for claim in transaction.claims:
if claim.claim.patient_control_number == claim_id:
return claim

assert False, f'Claim with ID {claim_id} not found'
Copy link
Author

Choose a reason for hiding this comment

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

Added this to be easier to hard check specific values in the test suite.

Choose a reason for hiding this comment

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

This is great 👍

@rposborne rposborne changed the title Adds LX and Ref for transactions Adds LX and REF for transactions Sep 27, 2024
Copy link

@curtisim0 curtisim0 left a comment

Choose a reason for hiding this comment

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

Some thoughts, otherwise, totally fine to keep the 🚂-🚋-🚋-🚋 moving!

return ref

@property
def production_date(self) -> datetime.datetime:

Choose a reason for hiding this comment

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

Suggested change
def production_date(self) -> datetime.datetime:
def production_date(self) -> datetime.date:

Choose a reason for hiding this comment

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

right?

Copy link
Author

Choose a reason for hiding this comment

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

Right now it's parsed to a datetime.

Comment on lines +130 to +133
def reference_identification_number(self) -> Optional[ReferenceSegment]:
for ref in self.references:
if ref.qualifier == 'Reference Identification Number':
return ref

Choose a reason for hiding this comment

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

I trust that this is the way. If we have multiple refs, are we wanting to return the moment it hits that value? or should there be a more "hardened" rule on the return. Just checking not ⚔️

Comment on lines +23 to +24
# https://www.cms.gov/medicare/billing/electronicbillingeditrans/downloads/835-flatfile.pdf
'EV': 'Reference Identification Number',

Choose a reason for hiding this comment

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

o my gracious linking these out would be lovely

Comment on lines +2 to +5
"python.testing.pytestArgs": ["tests"],
"python.testing.unittestEnabled": false,
"python.testing.pytestEnabled": true
"python.testing.pytestEnabled": true,
"python.analysis.extraPaths": ["./tests"]

Choose a reason for hiding this comment

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

Why we committing vscode settings?


elif identifier in cls.terminating_identifiers:
return transaction, segments, segment

else:
segment = None
message = f'Identifier: {identifier} not handled in transaction loop.'
message = f'Identifier: {identifier} not handled in transaction loop. {segment}'

Choose a reason for hiding this comment

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

Freaking neeeeeeeeed a better pattern here. But yes, this is gud 👍

segment = split_segment(segment)

self.identifier = segment[0]
self.number = segment[1]

Choose a reason for hiding this comment

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

fo sho

Comment on lines 6 to +7
sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), '..')))
sys.path.append(os.path.join(os.path.dirname(__file__), 'utils'))

Choose a reason for hiding this comment

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

We are not pytest.ini for backwards compatibility and what not, is that correct?

Choose a reason for hiding this comment

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

ironically these are all pytest fixtures 🤔

Comment on lines +38 to +44
def get_claim_by_control_number(transaction_set, claim_id):
for transaction in transaction_set.transactions:
for claim in transaction.claims:
if claim.claim.patient_control_number == claim_id:
return claim

assert False, f'Claim with ID {claim_id} not found'

Choose a reason for hiding this comment

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

This is great 👍

@rposborne rposborne merged commit 1210598 into main Sep 28, 2024
5 checks passed
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.

2 participants