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

Develop #1

Closed
wants to merge 28 commits into from
Closed

Develop #1

wants to merge 28 commits into from

Conversation

kevingill1966
Copy link

This code is ready for review / discussion. I set the version to 1.3.0a1.

Non-backward compatible changes

Changed the numbering of fields in the MSH segment. This breaks older code.
Parse all the elements of the message (i.e. down to sub-component). The inclusion of repetitions will break older code.

Others:

implemented a basic escaping mechanism
new constant ‘NULL’ which maps to ‘”“’
new isfile() / splitfile() functions to identify file (FHS/FTS wrapped messages)
new mechanism to address message parts via a symbolic name

Tests are updated
Docs are updated

johnpaulett and others added 28 commits August 30, 2011 19:49
http://wiki.medical-objects.com.au/index.php/Hl7v2_parsing

1.  Repetitions now supported
2.  Sub-components now supported
correct output for the separator fields. May not be backwards
compatible.
batching).

FHS / FTS mark the start and the end of a batch of messages. From the
parsing perspective, we just need to parse the file. The application
can take responsibility for the messages, I think.
for informational use. The functionality is defined in the base class.
an envelope for messages, not an alternative header.
return an empty string (assume that optional non-provided value was truncated from the message) .
Add an empty element in position 0 to index from 1. From now on, the Y in msg[N][Y][N][N][Y] are using a numbering that's compliant with HL7.
Add an empty element in position 0 to index from 1. From now on, the Y in msg[N][Y][N][Y][Y] are using a numbering that's compliant with HL7.
Add an empty element in position 0 to index from 1. From now on, the Y in msg[N][Y][Y][Y][Y] are using a numbering that's compliant with HL7. The last N left (index of the segment in the message) is not commonly used in HL7, it will thus be left as a 0-indexed number.
… empty element for compatibility with HL7 spec numbering
Numbering that respects the HL7 specifications
@e2jk
Copy link

e2jk commented Dec 2, 2012

Hey John, could you have a look at this Pull Request from Kevin that adds support for all elements of an HL7 message, down to the sub-component level?
Due to changes in numbering, this is backwards incompatible with previous versions, but in order to support those deeper levels there is no way around making backwards incompatible changes. Therefore, merging it with your main branch early enough is necessary to avoid more pain to future users ;)

@johnpaulett
Copy link
Owner

c@e2jk & @kevingill1966 -- Sorry for the delay on this, I agree it is important to get the changes in as soon as possible. I did spend some time reviewing previously and will try to dedicate some time to merge in.

@e2jk
Copy link

e2jk commented Jan 24, 2013

Hi John, did you find any issues when reviewing the merge request?

@e2jk
Copy link

e2jk commented Apr 2, 2013

Hi John, any issues reviewing the code?

@e2jk
Copy link

e2jk commented Apr 25, 2013

Hey John, sorry for insisting: have you had a chance to finish review the changes?

@gniemetz
Copy link

Hi John!

I am also interested in this.

regards
Gerd

@kevingill1966: After a short test with
import hl7

message = u'MSH|^~&|^2.16.840.1.113883.2.16.3.1.3.100.0^ISO|^2.16.840.1.113883.2.16.3.1.3.100.1^ISO|2.16.840.1.113883.2.16.3.1.3.100.990.1.1.2.11^1.3.6.1.4.1.21998.99.1.1.1.6^ISO|2.16.840.1.113883.2.16.3.1.3.100.990.1.1.2.11^1.3.6.1.4.1.21998.99.1.1.1.3^ISO|20130723141316||ADT^A31^ADT_A05|4ACQ8NAAFYOIC6V5GYN|T|2.5'

h = hl7.parse(message)

print h

i receive

[[[u'', u'MSH'], [u'', u'|'], [u'', u'^~&'], [u'', [u'', [u'', u''], [u'', u'2.16.840.1.113883.2.16.3.1.3.100.0'], [u'', u'ISO']]], [u'', [u'', [u'', u''], [u'', u'2.16.840.1.113883.2.16.3.1.3.100.1'], [u'', u'ISO']]], [u'', [u'', [u'', u'2.16.840.1.113883.2.16.3.1.3.100.990.1.1.2.11'], [u'', u'1.3.6.1.4.1.21998.99.1.1.1.6'], [u'', u'ISO']]], [u'', [u'', [u'', u'2.16.840.1.113883.2.16.3.1.3.100.990.1.1.2.11'], [u'', u'1.3.6.1.4.1.21998.99.1.1.1.3'], [u'', u'ISO']]], [u'', u'20130723141316'], [u'', u''], [u'', [u'', [u'', u'ADT'], [u'', u'A31'], [u'', u'ADT_A05']]], [u'', u'4ACQ8NAAFYOIC6V5GYN'], [u'', u'T'], [u'', u'2.5']]]

You see, many "blank" entries, what's going wrong here?

Thanks,
Gerd

@kevingill1966
Copy link
Author

I haven't looked at this for a while.

The effort I made here is to be able to reference the message elements via the spec coding. The numbering is not compatible with previous versions of the library, hence the problems merging it in.

h['MSH.7']
u'20130723141316'

h['MSH.9.1.1']
u'ADT'
h['MSH.9.1.2']
u'A31'
h['MSH.9.1.3']
u'ADT_A05'

@e2jk
Copy link

e2jk commented Aug 9, 2013

Please reopen this pull request, the changes are valid and make this library able to handle HL7 elements lower than fields. These changes are backwards compatible, but I don't see "problems" merging it, just non-responsiveness from John. We all are busy, I think everybody understands that, so let's give John another chance to improve this useful library!

@e2jk
Copy link

e2jk commented Aug 9, 2013

@gniemetz The "blank" entries are due to the way HL7 numbers their elements. Python lists are 0-based, while HL7 elements are 1-based. In order to make 1-based Python lists, every first (i.e. number 0) element is empty.

@gniemetz
Copy link

@e2jk Thanks for explanation

@e2jk
Copy link

e2jk commented Oct 25, 2013

I'd like, once again, to ask for the reopening of this pull request, as it is still valid.
These changes allow to go deeper in the structure of HL7, which does not stop at messages, segments and fields, but repetitions and subcomponents are an integral and important bit of the syntax.

@kevingill1966
Copy link
Author

Hi Emilien,

First of all congratulations on you perserverance.

I made this pull request 2 years ago when I was integrating a system
via HL7, and really have not been active at HL7 development since then.
It would be remiss of me to represent myself otherwise. That being
said, I hope to do some HL7 integration in the new year.

I have not considered this work in two years. Are you working in this
field? Can you articulate a roadmap for this module? If there is a
future for this work, I am open to being involved in a fork of John's
work. That is of course the point of open-source.

Kevin Gill
[1]kevin@movieextras.ie
[2]www.movieextras.ie

On Fri, Oct 25, 2013, at 07:06 PM, Emilien Klein wrote:

I'd like, once again, to ask for the reopening of this pull request,
as it is still valid.

These changes allow to go deeper in the structure of HL7, which does
not stop at messages, segments and fields, but repetitions and
subcomponents are an integral and important bit of the syntax.

Reply to this email directly or [3]view it on GitHub.
[SHfyfA6oEvpyhnMri7RpvAMd3lKiobjMRddfKfhVj3vImYjpMWAS7n3AJ46UC0V7.gif]

References

  1. mailto:kevin@movieextras.ie
  2. http://www.movieextras.ie/
  3. Develop #1 (comment)

@johnpaulett
Copy link
Owner

First off, let me express my apologies for not giving this pull request sufficient attention. It has languished too long, and that is my fault. I truly appreciate the work that everyone has contributed to this request.

I actually started the process of integrating this pull request several times and found it challenging. I actively use this module on a rather large code base and porting to new nested syntax has been significant work.

I honestly and truly would like to integrate this and I will spend some time reviewing again. A few notes so far:

  • At this point it is likely safe to drop support for < python 2.6. Several months ago I started on a 3.x port, I would rebase that work onto this PR.
  • I find the list-based slice syntax to be much less useful now it requires 3-6 sets of brackets. I understand the new syntax closer aligns with the HL7 spec, but feels much less pythonic (e.g. to iterate through all OBX-5 fields in a message requires string concatenation to build the key). I won't block the PR because of this, but if anyone has ideas on a simple-pythonic means of access, I would love to hear them (I'll think about this more too).

I am going to work on porting my application that depends upon python-hl7 and I may have additional notes. I will try to get this done by November 4th. That said, if others see a different path for this library I understand, github / pip make it easy to go in a different direction.

@johnpaulett johnpaulett reopened this Oct 25, 2013
@e2jk
Copy link

e2jk commented Oct 26, 2013

Hey @kevingill1966 and @johnpaulett, glad to read your reactions.
I do agree with John, the way the subcomponents, etc. is currently implemented is really not pythonic. It does the job but looks rather bad. And I am to blame for that ;)

I guess what would be needed is some kind of subclass of a python list, which would be 1-based instead of 0-based.. That way, you could use standard HL7 notation but still display the content in the Python way.

Another question is that of repetitions: when there is only one repetition, its number is ignored/assume, e.g. you would mention OBX-3, not OBX-3.1. But in the data model, we can't assume that, so we must always store the value in a first repetition, and return OBX-3.1 when OBX-3 is asked for. But what if you want to ask the first subcomponent of the first repetition? You would ask for OBX-3.1, but really that is OBX-3.1.1. Not sure if I'm clear in my explanation, but that is the point: OBX-3.1 could either mean the first repetition in OBX-3, or the first component of the first repetition of OBX-3...

How to best approach this, I'm not sure.

@johnpaulett
Copy link
Owner

Just wanted to give everyone an update.

I have been merging the code and porting my code base. I think I have a few small adjustments that I'm working to add into this PR. Hoping to get it done and pushed out this week.

@e2jk
Copy link

e2jk commented Nov 17, 2013

Hi John, how is the merging and code updating going?

@e2jk
Copy link

e2jk commented Dec 2, 2013

Hi John, in the coming days we would like to start implementing a new HL7 module for GNU Health, which will require support for subcomponents, repetitions, etc. i was wondering if you would have any chance of finalizing this merge to your master branch, so that we can use the "official" library instead of @kevingill1966 's fork.
Please let us know.

@kevingill1966
Copy link
Author

Hi Emilien,

I am interested in Gnu Health, and your plans to implement a HL7
interface. I met Luis in Barcelona last month at the Tryton sprint and
I am hugely impressed by Luis and the project.

If you have any notes on your implementation (or planned
implementation) that you care to share, I would be interested in
reading them and tracking your work.

Good luck with the integration, unfortunately, I cannot offer to help
as I am over-committed on another project.

Kevin Gill
kevin@movieextras.ie
www.movieextras.ie

On Mon, Dec 2, 2013, at 12:11 AM, Emilien Klein wrote:

Hi John, in the coming days we would like to start implementing a
new HL7 module for [1]GNU Health, which will require support for
subcomponents, repetitions, etc. i was wondering if you would have
any chance of finalizing this merge to your master branch, so that
we can use the "official" library instead of [2]@kevingill1966 's
fork.

Please let us know.

Reply to this email directly or [3]view it on GitHub.
[SHfyfA6oEvpyhnMri7RpvAMd3lKiobjMRddfKfhVj3vImYjpMWAS7n3AJ46UC0V7.gif]

References

  1. http://health.gnu.org/
  2. https://github.com/kevingill1966
  3. Develop #1 (comment)

@e2jk
Copy link

e2jk commented Dec 2, 2013

I have forwarded @kevingill1966 's email to the health-dev mailing list.
Kevin, I hope to give a quick presentation about GNU Health and HL7 in the coming days [0] to kick of this project. Feel free to attend if time permits!

+Emilien

[0] https://lists.gnu.org/archive/html/health-dev/2013-12/msg00007.html

@rectalogic
Copy link
Contributor

I don't want to derail merging this pull request with another patch right now, but it would be nice to also have a non-string based API to get/set fields. This would make it easier to iterate repeating fields without having to build a string key each time, but still get the safety and forward/backward compatibility features of the key based API. e.g.

def extract_field(self, SEG, SEGn=1, Fn=1, Rn=1, Cn=1, SCn=1):

So you could do something like this to get "PID.3.1.4", "PID.3.2.4" and "PID.3.3.4":

for i in range(3):
    message.extract_field("PID", Fn=3, Rn=i+1, Cn=4)

@e2jk
Copy link

e2jk commented Dec 31, 2013

Hi @johnpaulett , how are your porting efforts going?
@rectalogic I do think that it's best to keep your suggestion as a next step ;) Let's get this 2 year-old change merged first, then see how to further improve it.

@waynew
Copy link

waynew commented Jan 31, 2014

Hey @johnpaulett I basically have the same question as @e2jk here - how are the porting efforts going? I basically just ran the repo through 2to3, which seems to work, though I don't know how well. Are you too busy to properly maintain this thing, or have you just needed a bit of a nudge to get back in?

@e2jk
Copy link

e2jk commented Jan 31, 2014

Hi @kevingill1966, I notice you've closed the pull request, but I don't think you've merged the code. was that a conscious decision, or just an omission?
In the latter case, please let us know what motivated that decision ;)
Thanks for your work on this library!

@johnpaulett
Copy link
Owner

@e2jk & @kevingill1966 . That closure of the pull request may have been a github blip I triggered (deleted develop branch to simplify and only use master). I added travis & tox support. I still consider this item open, but am unable to re-open this issue via github's interface.

@e2jk
Copy link

e2jk commented Jan 31, 2014

Maybe @kevingill1966 needs to make a new pull request pointed towards the master branch?

@kevingill1966
Copy link
Author

I am only peripherally following this. I re-issued the pull request as
suggested by Emilien Klien.

Please let me know if that worked as expected. I am no git guru.

Kevin Gill
kevin@movieextras.ie
www.movieextras.ie

On Fri, Jan 31, 2014, at 07:41 PM, Emilien Klein wrote:

Maybe [1]@kevingill1966 needs to make a new pull request pointed
towards the master branch?

Reply to this email directly or [2]view it on GitHub.
[575098__eyJzY29wZSI6Ik5ld3NpZXM6QmVhY29uIiwiZXhwaXJlcyI6MTcwNjczMDA2OS
wiZGF0YSI6eyJpZCI6NDg2NDEwOX19--4759187604ee8eb8f665b2acad208a04b83bc7d
6.gif]

References

  1. https://github.com/kevingill1966
  2. Develop #1 (comment)

@johnpaulett
Copy link
Owner

(off-topic for this PR)
@waynew Added support for Python 3.3+ on feature/python3-support (40a7716). It's not as easy as just running through 2to3. The Python 3 unicode vs byte string forces python-hl7 to be more explicit about when it is dealing with binary versus unicode data. This branch allows Python 2.6+ and Python 3.3+ on a single codebase and passes all the tests.

@johnpaulett
Copy link
Owner

My sincere apologies for how long this PR has lingered. It represented a very valuable improvement to supporting HL7 properly. @kevingill1966 and @e2jk I sincerely appreciate your work on this.

Thanks to work by @rectalogic, I have merged this code into master and cut a 0.3.0 release.

Part of the hold up on merging had been finding time to port a fairly substantial application that uses this library, to the new nested format. Now that this work has been completed, I will be more comfortable cutting more frequent releases to python-hl7.

I wrote a bit of compatibility code to help the transition for my application, perhaps it is useful for others (I have not included inside the library at this point):

def access(segment, field, component=None, repetition=0, subcomponent=0):
    """
    Old-style field-component access within a segment, that assumes the first
    repetition (if present) and subcomponent are what is desired.
    """
    item = segment[field]
    if isinstance(item[repetition], hl7.Repetition):
        item = item[repetition]

    if component is None:
        return item

    item = item[component]
    get_subcomponent = lambda value: (
        value if isinstance(value, basestring) else value[subcomponent]
    )
    if isinstance(component, slice):
        item = [get_subcomponent(i) for i in item]
    else:
        item = get_subcomponent(item)

    return item

Again, sorry that this took so long, but I hope after putting this massive change behind us, we can more quickly iterate on improving the library.

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.

None yet

6 participants