xtal posted:

I honestly don't know that much

Da Mott Man posted:

Its been a while working with sqlalchemy but if I remember correctly your id columns should have autoincrement=True and for speed of lookups you should have an index=True for any column you would use to select records.
You might also want to cascade delete child data if the patent is deleted for some reason.
I would also set the dateofbirth column to a DateTime instead of a string, it would be easier to query for patents of specific age or range of ages.

Hollow Talk posted:

I'm not a fan of pro-forma integer id columns. I see these columns often, and they are useless more often than not (auto-incrementing columns are only really useful if you want to check for holes in your sequence, i.e. for deleted data, and that can be solved differently as well).

Patient already has a primary key, so the id column is superfluous. For the other tables, keys depend on how often you expect to load new data. If this only gets new data once a day, just make the combination of MRN + date the table's primary key (i.e. a compound key) -- SQLAlchemy lets you set multiple columns as primary keys, and it just makes a combined key out of it. If you update more frequently than daily, I would change date to a timestamp, and use that. Realistically, depending on your analysis needs, you will probably end up joining and either selecting the full history for a single patient, or the current status for all patients (or a subset thereof), which this would cover either way. For LabValues, you might need MRN + date + name (assuming name is whatever was tested in the lab).

Essentially, I am of the opinion that primary keys should have meaning on their own, which sequences (i.e. what SQLAlchemy will most likely create under the hood if you have an auto-incrementing integer column and if the underlying database supports sequences) most often do not.

Edit: This is on top what has already been said. Also, foreign keys look good to me.

M. Night Skymall posted:

You should probably not use the MRN as the identifier throughout your database. It's generally best practice with PHI to isolate out the MRN into a de-identification table and use an identifier unique to your application to identify the patient. It's easy enough to do a join or whatever if someone wants to look people up by MRN, but it's useful to be able to display some kind of unique identifier that isn't immediately under all the HIPAA restrictions as PHI. Even if it's as simple as someone trying to do a bug report and not having to deal with the fact that their bug report must contain PHI to tell you which patient caused it. Not vomiting out PHI 100% of the time in error messages, things like that. Just make some other ID in the patient table and use that as the foreign key elsewhere.

Jose Cuervo posted:

To be clear, this is a research data set which lives on a HIPAA compliant server. It is currently a bunch of excel files that were pulled by the IT team who run the health system databases. I would like to turn the excel files into a database so that things are much more organized and I can learn about databases. Hence why I was asking for help.

Jose Cuervo posted:

Would you have this same concern given the use case (a research database where the only people which access to it are on the IRB protocol - currently just me for now)?

bobmarleysghost posted:

Is there a good library/framework for creating command line interactive programs?

Jose Cuervo posted:

Would the following change to the Diabetic class be along the lines of what you are suggesting?

Python code:

class Diabetic(Base):
    __tablename__ = 'diabetic'
    
    MRN = Column(Integer, ForeignKey('patients.MRN'), primary_key=True)
    date = Column(Date, primary_key=True)
    diabetic = Column(Boolean)    
    
    patient = relationship('Patient', back_populates='diabetic') 

Can you explain what advantage there is to having the compound key?

Bundy posted:

click.

Unless you mean TUI type apps in which case urwid is pretty solid.

M. Night Skymall posted:

It doesn't matter what the data is used for. The history of MRN as PHI is pretty dumb in my opinion, but per guidance from the government your MRN is as much PHI as your DOB or name. Having a de-identification table isn't a big deal, you can still store all your PHI in the patients table along with your new unique identifier. It's really *just* to remove the awkwardness of having everything in your DB keyed to a piece of PHI. I mean you're right, it's just you and it probably won't affect much now. But making good decisions about your schema is much..much easier now than it is later, and there's basically no way you will live to regret de-identifying your data in advance, and many ways you can live to regret spreading the MRN all over your database.

Anyone who's looking at this going "No no not python!" is just forcing this data to be sucked into a PowerBI tool instead. Don't make this poor person use PowerBI, that's just mean.

Edit: MRN is problematic because it's specifically listed as an identifier. If you have an internal ID in use for patients besides MRN that'd also be better than using the MRN directly. One more dumb thing about MRNs, they're specific to the hospital EMR so they aren't guaranteed to be unique.

Hollow Talk posted:

Yes, that would be it.

Essentially, the reason why I dislike auto-increment integer columns is that they serve no particular purpose, and they provide no guarantees -- if you want to enforce constraints (i.e. to avoid duplicate data if you run your tool against the Excel files more than once), auto-incrementing columns don't help, because the only constraint for the database is that the primary key is unique (which a sequence ensures), plus any foreign key relations if you specify them. If you insert the same data again, the original option will happily do so, given this data:

Python code:

with engine.begin() as connection:
    _patient = {
        "MRN": 1,
        "first_name": "Hollow",
        "last_name": "Talk",
        "date_of_birth": "1970-01-01",
    }
    _diabetic = [
        {"MRN": 1, "date": datetime.date(2021, 3, 23), "diabetic": False},
        {"MRN": 1, "date": datetime.date(2021, 3, 23), "diabetic": False},
    ]

    connection.execute(insert(Patient).values(_patient))
    connection.execute(insert(Diabetic).values(_diabetic))

If you define an auto-increment column, this will insert even though the diabetic data row is duplicated. If you instead define your primary keys according to your actual uniqueness logic, python will throw an exception for the above duplicate:

Bash code:

sqlalchemy.exc.IntegrityError: (sqlite3.IntegrityError) UNIQUE constraint failed: diabetic.MRN, diabetic.date
[SQL: INSERT INTO diabetic ("MRN", date, diabetic) VALUES (?, ?, ?), (?, ?, ?)]
[parameters: (1, '2021-03-23', 0, 1, '2021-03-23', 0)]
(Background on this error at: [url]http://sqlalche.me/e/14/gkpj[/url])

In most cases*, I prefer my inserts to fail as early as possible when constraints are violated, so that I can fix my insert/select logic, rather than clean up duplicate and/or otherwise broken data afterwards. This lets you express what can and cannot be inserted more than once, and you don't add any technical columns that don't really serve any analytical purpose (nor any other purpose, really). For this use case, using the auto-increment column as primary key and not using any primary key are functionally identical.

Unrelated: If you can avoid it, try to stick to lowercase column names, so use mrn instead of MRN. Many databases don't care, but for those cases where they do, this saves you from lots of quoting (or general incompatibility).

*: This does not apply for data warehousing, where constraints might not be feasible. Or if you use something like Amazon Redshift, because "we'll just ignore all constraints because that will make things faster" is certainly one way to speed up inserts.

M. Night Skymall posted:

You should probably not use the MRN as the identifier throughout your database. It's generally best practice with PHI to isolate out the MRN into a de-identification table and use an identifier unique to your application to identify the patient. It's easy enough to do a join or whatever if someone wants to look people up by MRN, but it's useful to be able to display some kind of unique identifier that isn't immediately under all the HIPAA restrictions as PHI. Even if it's as simple as someone trying to do a bug report and not having to deal with the fact that their bug report must contain PHI to tell you which patient caused it. Not vomiting out PHI 100% of the time in error messages, things like that. Just make some other ID in the patient table and use that as the foreign key elsewhere.

Jose Cuervo posted:

Got it. The case of trying to insert data which already exists in the database is something I was thinking about, and the way you suggested defining the compound key neatly solves that issue.

As a follow-on question, would you wrap the insert statement in a try-except block, and catch the "sqlalchemy.exc.IntegrityError" exception to deal with not crashing when trying to insert data which is already in the database?

Jose Cuervo posted:

I am on board with doing this, but I am having trouble wrapping my head around how I would accomplish this. Do I define a new column, say pid as the primary_key for the Patient model as below, and use pid as the foreign_key in the other models as below for the Diabetic model?

However, if I do this, then I need to keep a mapping between the MRN and the pid outside of the database, right? So now I have the MRN in two different places...

Jose Cuervo posted:

I have changed the column to DateTime, that was a typo.
Thanks for the keyword 'cascade' - that allowed me to look up what I wanted. I think the new definition of Patient with cascade='all, delete' will accomplish the desired behavior, right?

Zoracle Zed posted:

Anyone else noted how awful it is googling for anything python-related these days? SEO means the first couple pages of results are all geeksforgeeks.com and other awful beginner tutorial spam. (Which, like, even for beginners, seem bad.)

Anyway, here's my actual question: in matplotlib.pyplot.plot there's this concept of a format string, so 'r--' means a red dashed line, etc. Somewhere in matplotlib, I assume, there has to be a format string parser that takes in 'r--' and spits out {color: 'r', linestyle: '--'} or whatever. Point me in the right direction, please? Whenever I write convenience plotting methods I end up fighting to balance the convenience of the very-compact format string style and actual structured argument handling.

Framboise posted:

As/for someone with very basic programming knowledge who wants to look into programming as a career: Is this Udemy course https://www.udemy.com/course/100-days-of-code/ worth looking into? Seems like a good deal for 63 hours of content, but if it's not good I don't want to put my time into it.*

* Specifically, it's on sale right now for $12.99 which sounds like a good deal, but that price for the bold claims they make sounds kinda sus to me.

Edit: there's also this one https://stacksocial.com/sales/the-2021-premium-python-certification-bootcamp-bundle from StackSocial that claims to be 41 hours of content worth $2585 for only 34.99 and again, sus.

DoctorTristan posted:

You want to know the kwargs alternative to the format string, or the location of the parser module? If the latter why not just call it with an invalid string and see where the exception gets thrown from?

Zoracle Zed posted:

Anyone else noted how awful it is googling for anything python-related these days? SEO means the first couple pages of results are all geeksforgeeks.com and other awful beginner tutorial spam. (Which, like, even for beginners, seem bad.)

punished milkman posted:

I can’t tell if I’m just getting old and pining for the imaginary good ol’ days or what, but I definitely feel this. It goes beyond anything Python related, Google takes your search string and returns mostly trash for just about any subject now.

Anyway, I usually use site:stackoverflow.com with my Google searches which helps filter out the fluff (and potentially some other good stuff). The main annoyance here is that the results are not sorted by date so you’ll get answers from like 2009 which usually aren’t all that helpful.

Foxfire_ posted:

You have a meaningless-outside-your-application patient ID that uniquely identifies a patient. All of your deidentified tables reference that for knowing what patient a row is about. Then you have a table that maps from the the patient ID to all the PHI about that patient (MRN, names, DOB, ...). That table could be either inside the same database or elsewhere if you wanted to segregate the dataset into PHI and deidentified parts.

You might also need multiple PHI tables for some stuff, like if you want to store multiple different-system/different-site MRNs for one actual person. For each table in the schema, you should be able to classify it as PHI or not and shouldn't be mixing PHI and not-PHI together in the same table. If you've done stuff right, you ought to be able to hand someone all your non-PHI tables and they'd be able to follow information about a particular subject, but not have any PHI about them

Jose Cuervo posted:

I guess I am struggling to understand how I would structure this. Based on what has been said I would only need the MRN of the patient and the health system the patient belongs to to uniquely identify them. In the definition of the Patient model below I have set the primary key to be a compound key (pid, mrn, and health_system).

Python code:

class Patient(Base):
    __tablename__ = 'patients'

    mrn = Column(Integer, primary_key=True)
    health_system = Column(String, primary_key=True)
    pid = Column(Integer)
    first_name = Column(String)
    last_name = Column(String)
    date_of_birth = Column(Date) 

    diabetic = relationship('Diabetic', 
                            back_populates='patient', 
                            cascade='delete, all')


class Diabetic(Base):
    __tablename__ = 'diabetic'
    
    pid = Column(Integer, ForeignKey('patients.pid'), primary_key=True)
    date = Column(Date, primary_key=True)
    diabetic = Column(Boolean)  
    
    patient = relationship('Patient', back_populates='diabetic')   

However,
1. Where do I get the value for pid from? Do I need to generate that separately each time I want to enter a new patient?
2. Suppose I now have data from a given patient (I know their MRN and health system). In order to get their pid, do I first perform a search in Patient to find the pid, and then use the pid to insert their data into the relevant tables?


EDIT: If the above is correct, I just tried adding data to the database while adding "autoincrement=True" to the pid:

Python code:

class Patient(Base):
    __tablename__ = 'patients'

    health_system = Column(String, primary_key=True)
    mrn = Column(Integer, primary_key=True)   
    pid = Column(Integer, autoincrement=True)
...

However, this does not automatically add a new unique integer to the pid column each time a new patient is added. Not sure what I am doing incorrectly here.

vikingstrike posted:

However, this does not automatically add a new unique integer to the pid column each time a new patient is added. Not sure what I am doing incorrectly here.

Jose Cuervo posted:

Hollow Talk posted:

Postgres even has a UUID type, but sqlite doesn't support that.

QuarkJets posted:

Simplest way to implement a context manager: open a single session outside of your for loop with one. Example:

Python code:

Session = sessionmaker(bind=engine)
with Session() as session:
    for MRN, MRN_df in esa.groupby('MRN'):
        add_dose_data('Health_System', MRN, MRN_df, session)

You were opening 1 session per patient, and you weren't closing them. With a context manager you can just open 1 session, do all of your work, and then the context manager closes it for you. Or if you want 1 session per patient, you can just use the context manager inside of the for loop instead of outside of it.

To get even finer-grain than that, you could pass the Session (e.g. the session maker) and create sessions on the spot with a context manager using Session.begin(); then when the context closes the transaction is also committed for you, or a rollback occurs if an exception is raised. Much more slick than doing your own commits or rollbacks. Example:

Python code:

# Note: this function expects a session maker now
# e.g. Session instead of Session()
# You should probably call it something else for easy reading, like session_maker
def check_add_patient(provider, MRN, df, Session):
    ...
    with Session.begin() as session:
        session.add(ddb.Patient(mrn=int(MRN), provider=provider, 
                                pid=session.query(ddb.Patient).count() + 1, 
                                first_name=df.iloc[0]['First_Name'],
                                last_name=df.iloc[0]['Last_Name']))
    # Session is committed and closed here, no need commit
    ...