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

0.0.9 release #487

Closed
jbteves opened this issue Dec 11, 2019 · 35 comments
Closed

0.0.9 release #487

jbteves opened this issue Dec 11, 2019 · 35 comments
Labels
discussion issues that still need to be discussed
Milestone

Comments

@jbteves
Copy link
Collaborator

jbteves commented Dec 11, 2019

With #482 merged, should we cut a new release?
Credit to @tsalo for asking in Gitter; issue opened for record-keeping.

Emoji vote below: 👍 for yes and 👎 for no.

@jbteves jbteves added the discussion issues that still need to be discussed label Dec 11, 2019
@emdupre
Copy link
Member

emdupre commented Dec 11, 2019

Haha I can't tell what both an upvote and a downvote mean, @tsalo 😆

I think we should wait until #467 is merged, personally. That's my nights-and-weekends plan for the next week or two.

@tsalo
Copy link
Member

tsalo commented Dec 11, 2019

I was just adding them so people could click on one vs. the other. Initiating a vote, basically.

@dowdlelt
Copy link
Collaborator

#331 Seems like it could also be brought in to the fold. Conceptually, I believe it is the right approach.

@emdupre
Copy link
Member

emdupre commented Dec 11, 2019

Initiating a vote, basically.

Maybe we can edit @jbteves top level comment to be an emoji vote, then ? As-is you can't vote, @tsalo .

@jbteves
Copy link
Collaborator Author

jbteves commented Dec 12, 2019

@emdupre it looks like voting works now but maybe someone else should check.

@eurunuela
Copy link
Collaborator

eurunuela commented Dec 12, 2019

Voting works for what I can see. Also, I agree with #467 getting merged before the release.

@jbteves
Copy link
Collaborator Author

jbteves commented Dec 12, 2019

Thanks for checking @eurunuela. We seem to have a few cleanup issues as well that might be good to get in.

@tsalo
Copy link
Member

tsalo commented Dec 12, 2019

Given that #467 will probably take a while, should we plan for an early- to mid-January release?

@tsalo
Copy link
Member

tsalo commented Dec 12, 2019

@jbteves since there are a handful of minor issues/PRs we want to address in addition to the dynamic reports, how about we make a milestone to track them.

@tsalo tsalo added this to the 0.0.9 milestone Dec 12, 2019
@jbteves
Copy link
Collaborator Author

jbteves commented Dec 12, 2019

Sounds good to me!

@smoia
Copy link
Collaborator

smoia commented Dec 12, 2019

I'm so sorry: I know I might be the reason for which #467 is not advancing. I thought I would have been able to finish the review before starting OHBM submission mode, but unluckily that was not the case.
If you want to substitute me as reviewer to speed up the process and have that PR for OHBM submission, I'm definitely in favour of that. Otherwise I'll try to do my best, but I can't assure I will be able to tackle this before the 19th of December.
Very very sorry.

@jbteves
Copy link
Collaborator Author

jbteves commented Dec 13, 2019

@smoia there is no need to apologize. This is a side project which nobody is really funded specifically to work on/manage. Don't let that academic guilt get the better of you ; - )

This really shouldn't be rushed, as it's a very major change that will substantially alter how the community interacts with tedana, so don't! Make sure you get your submission in and then give the PR your attention when you have the time. If you know of another reviewer then great, if not then no worries. Would you like us to ping you sometime after the 19th?

@tsalo
Copy link
Member

tsalo commented Feb 27, 2020

Given that there is one bug in tedana that likely adversely impacts fMRIPrep (#473), as well as proposed or merged changes that will break compatibility with fMRIPrep (#540), I think we should try to get 0.0.9 released and fMRIPrep updated right before the next planned fMRIPrep release, assuming it's not going to happen in the next couple of days.

I know that we want to wait until #467 is merged, but it doesn't seem like that PR is ready just yet, and I feel like the performance issues and breaking changes with fMRIPrep are more pressing.

EDIT: I realized that #473 actually probably doesn't affect fMRIPrep, given that fMRIPrep only calls the t2smap CLI. I still think it would be a good idea to deal with that issue before 0.0.9, since it impacts users of the denoising workflow, as well as Python users.

@handwerkerd
Copy link
Member

@mvaziri recently asked me about which version of tedana an end user should use right now. I'm realizing that the bug fixes from the PCA/ICA step aren't in 0.08 so we shouldn't recommend that to end-users right now. As of now, my advice will be to use the most recent code rather than pip install tedana I know there are a lot of moving parts in new version releases, but I might suggest leaning towards getting that PCA/ICA fix into a released 0.09 version sooner rather than later.

@tsalo
Copy link
Member

tsalo commented Mar 25, 2020

@handwerkerd Regarding the PCA/ICA fix, are you referring just to #490?

@handwerkerd
Copy link
Member

I'm referring to #435 and some of the subsequent changes that cleaned it up. There are still things that can be improved, but the maPCA algorithm isn't in the release version of the code yet and it probably should be.

@tsalo
Copy link
Member

tsalo commented Apr 1, 2020

Okay gotcha. Now that our biggest bug is squashed (i.e., #555), I think we should address #540 and then make the release. We can postpone the other issues tied to 0.0.9 until 0.0.10/0.1.0 (depending on what we go with). Does that sound good?

@eurunuela
Copy link
Collaborator

Sounds good to me! I still haven't had time for the dynamic reports, sorry. I might work on it this Friday or Saturday.

@jbteves
Copy link
Collaborator Author

jbteves commented Apr 17, 2020

@ME-ICA/tedana-devs
This is what I propose for adding to the top of the release drafter:

This release contains a number of breaking, fixing, and useful changes.
We encourage users to review our heavily expanded documentation at
tedana.readthedocs.io

Bug Fixes:
- PCA has been overhauled to a new and more reliable method, averting a known
  bug where too many PCA components would be selected.
- Environments are not coerced to single-threaded computation after calling
  tedana.
- Fixed variance-explained outlier detection problem where first value was 
  always NaN and variance explained was always negative.
- Fixed component table loading bug that resulted from unexpected pandas
  behavior.
- Fixed bug where the wrong number of echoes would be allocated in-program.
- Fixed bug where only selecting one component would cause an error.
- Correctly incorporate user-supplied masks in T2* workflow.
- Fixed bug in PAID combination where mean of data would be used instead of SNR.

Breaking Changes:
- Log files are now by datetime, allowing multiple runs to have systematic
  naming.
- Filenames for decomposition and metric maps are now BIDS
  derivative-compatible. Please see documentation for the full list of new
  filenames.
- Component tables are now in .json format
- Changed tab-separated files from .txt to .tsv file extension.
- Removed the --sourceTEs option.
- T2* maps are now in seconds rather than milliseconds.
- --mle option is now deprecated.

Changes in Defaults:
- New PCA algorithm is default, please see documentation for more information.
- Clustering is now bi-sided rather than two sided (positive and negative
  clusters are now grouped separately).
- Static png images are now the default; use --nopng to avoid this.
- Files are now gzipped by default.

New Features:
- Massively expanded documentation, please see tedana.readthedocs.io to view
  the updated usage help, multi-echo background, developer guidelines, and
  API documentation.
- New PCA decomposition algorithm (default).
- Adds the --out_dir argument to t2smap workflow to choose what directory files are written
  to.
- t2smap workflow is now fmriprep compatible
- Added --t2smap argument to allow you to supply a precalculated T2* map.


Thanks to Logan Dowdle, Elizabeth DuPre, Cesar Caballero Gaudes, Dan
Handwerker, Ross Markello, Isla, Joshua Teves, Eneko Urunuela, 
Kirstie Whitaker, and to the NIH Section on Functional Imaging Methods
for supporting the tedana hackathon and the NIH for supporting the AFNI Code Convergence, where much of the work in this release was done.

@handwerkerd I had thought it was SFIM specifically that hosted the hackathon but let me know if that's incorrect and we can modify the acknowledgement.

@tsalo
Copy link
Member

tsalo commented Apr 17, 2020

Thank you for doing this.

I would add that component tables are now jsons (tied in with the BIDS filenames).

Also, for - Changed tab-separated files from .csv to .tsv file extension., it should be .txt to .tsv.

@eurunuela
Copy link
Collaborator

I feel like the new PCA could be in the Breaking Changes and New Features sections too haha. LGTM, thanks @jbteves!

@jbteves
Copy link
Collaborator Author

jbteves commented Apr 17, 2020

Okay, @tsalo I think I've incorporated those changes into my edit, let me know if that looks good to you.
@eurunuela I went ahead and added it there, too. It probably can't be said too many times.

@eurunuela
Copy link
Collaborator

@jbteves Of course it can’t be said too many times, I was just kidding :P

@mvaziri
Copy link
Collaborator

mvaziri commented Apr 17, 2020

Really looking forward to this release. Great job everyone :-)

@tsalo
Copy link
Member

tsalo commented Apr 24, 2020

@jbteves the release notes will also need to include info for #358 and #560. And #557 if it ends up being merged.

@jbteves
Copy link
Collaborator Author

jbteves commented Apr 29, 2020

Sorry for missing this before, but please review the release notes edit.

@tsalo
Copy link
Member

tsalo commented Apr 29, 2020

In - Fixed bug where mean of data would be used instead of SNR. I would just clarify that it's a bug in the PAID combination method specifically.

In - Adds the --out_dir argument to choose what directory files are written to. I would say that the argument is being added to the t2smap workflow, since the tedana workflow already had it.

Other than that, everything looks great to me!

@jbteves
Copy link
Collaborator Author

jbteves commented Apr 29, 2020

@tsalo better?

@tsalo
Copy link
Member

tsalo commented Apr 29, 2020

@jbteves Perfect, thank you!

@jbteves
Copy link
Collaborator Author

jbteves commented Apr 29, 2020

Okay, if we're all agreed, who would like to do the honor of releasing? Or does @emdupre specifically need to?

@handwerkerd
Copy link
Member

Tweak the text to clarify support, "NIH Section on Functional Imaging Methods
for supporting the tedana hackathon and the NIH for supporting the AFNI Code Convergence"

Otherwise looks great. Thank you for getting this done!

@jbteves
Copy link
Collaborator Author

jbteves commented Apr 29, 2020

@handwerkerd is that better?

@jbteves
Copy link
Collaborator Author

jbteves commented May 1, 2020

@ME-ICA/tedana-devs please view the draft here:
https://github.com/ME-ICA/tedana/releases/tag/untagged-c0a5ac7144aa4c67bfab
I believe somebody would just need to push the "publish" button. Release-drafter could be employed but I think that the changes would all drown in each other. Perhaps someone more familiar with release-draft knows how to insert a header at the top.

@emdupre
Copy link
Member

emdupre commented May 1, 2020

Reviewing and then I'm on it 👍

@emdupre
Copy link
Member

emdupre commented May 5, 2020

OK, I needed to double-check on the Zenodo integration, so I've cut this first as an alpha release and it's available on PyPi now ! Users just need to run

pip install tedana==0.0.9a

to grab it. I'll copy over the release notes to the stable version, too. Going to close this since the release series has now launched, but please let me know if you have any other concerns, here !

Thanks everyone 💛

@emdupre emdupre closed this as completed May 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion issues that still need to be discussed
Projects
None yet
Development

No branches or pull requests

8 participants