Skip to content

WIP: 0.9#432

Merged
tyarkoni merged 116 commits intomasterfrom
sqlalchemy
May 14, 2019
Merged

WIP: 0.9#432
tyarkoni merged 116 commits intomasterfrom
sqlalchemy

Conversation

@tyarkoni
Copy link
Copy Markdown
Collaborator

@tyarkoni tyarkoni commented Apr 10, 2019

This is a major refactoring of the codebase that addresses #422 by replacing pretty much the entire existing storage/querying code with the SQLAlchemy ORM. I've tried to minimize API changes, but the nature of the changes here made that impossible in some places (e.g., as a trivial example, SQLAlchemy reserves the .metadata property on models, so we now have BIDSFile.get_metadata() instead).

Closes #430.
Closes #431.
Closes #422.
Closes #404.
Closes #398.
Closes #383 (I think).
Closes #232.
Closes #273.
Closes #346.
Closes #402.
Closes #435.

@adelavega
Copy link
Copy Markdown
Collaborator

One minor thing that is not working for me is that I used to pay layout.get(desc=None, ...) to retrieve only the original (not derivative) images. Passing desc=None now results in getting no files back (presumably because those files don't have a desc attribute at all).

@adelavega
Copy link
Copy Markdown
Collaborator

scope='raw' should do the trick though

@adelavega
Copy link
Copy Markdown
Collaborator

Another minor API change: AttributeError: BIDSImageFile object has no attribute named 'image'
When doing img.image

Otherwise all my neuroscout tests are passing!

@tyarkoni
Copy link
Copy Markdown
Collaborator Author

tyarkoni commented May 2, 2019

.image is gone (and so is .metadata); you now use getters for all those things (get_image(), get_metadata(), get_df(), etc.).

Glad to hear everything is passing; this was even smoother than 0.8!

@adelavega
Copy link
Copy Markdown
Collaborator

Yeah, great job keeping the API fairly stable.

@tyarkoni
Copy link
Copy Markdown
Collaborator Author

tyarkoni commented May 7, 2019

Any objections to merging this, @adelavega @effigies @yarikoptic?

@effigies
Copy link
Copy Markdown
Collaborator

effigies commented May 7, 2019

I haven't had a chance to figure out what's going on with FitLins reports, but I'll just require pybids<0.9 for my next release.

@adelavega
Copy link
Copy Markdown
Collaborator

LGTM!

@hjmjohnson
Copy link
Copy Markdown
Contributor

@tyarkoni Any progress on this? It seems that pybids is stalling a bit. :)

@tyarkoni
Copy link
Copy Markdown
Collaborator Author

tyarkoni commented May 8, 2019

@hjmjohnson yes, plenty of progress—see the commit history for the branch.

@tyarkoni
Copy link
Copy Markdown
Collaborator Author

tyarkoni commented May 8, 2019

Also, @hjmjohnson, your clock may be off, because you're commenting from the future ;)

@tyarkoni
Copy link
Copy Markdown
Collaborator Author

Okay, unless there are any last-minute objections, I'm merging this today. If we catch any issues after the fact, we can release 0.9.1 in short order.

@yarikoptic
Copy link
Copy Markdown
Collaborator

sorry for the delay -- gimme a day to test just a bit more

@tyarkoni
Copy link
Copy Markdown
Collaborator Author

Okay no worries.

@tyarkoni tyarkoni merged commit 76cfcf5 into master May 14, 2019
@effigies effigies deleted the sqlalchemy branch September 9, 2020 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment