-
Notifications
You must be signed in to change notification settings - Fork 125
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
enh: add telemetry support #369
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't tried yet, but great to see it coming!
heudiconv/cli/run.py
Outdated
if latest and 'version' in latest: | ||
print("Your version: {0} Latest version: {1}".format(__version__, | ||
latest["version"])) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if this would be the right place to inform. I would like heudiconv --version
output remain containing only the current version of the heudiconv
heudiconv/cli/run.py
Outdated
except RuntimeError as e: | ||
print("Could not check for version updates: ", e) | ||
else: | ||
if latest and 'version' in latest: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't it compare __version__
to the latest
somehow?
heudiconv/cli/run.py
Outdated
try: | ||
latest = etelemetry.get_project("nipy/heudiconv") | ||
except RuntimeError as e: | ||
print("Could not check for version updates: ", e) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a >90% guarantee that etelemetry
wouldn't crash for some other (IO/Syntax/whatever) reason? ;-) I would have caught all other exceptions (eg. except Exception as e
) and issued a warning but kept going
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use lgr.warning
here?
heudiconv/info.py
Outdated
@@ -14,6 +14,7 @@ | |||
'nipype>=1.0.0', | |||
'pathlib', | |||
'dcmstack>=0.7', | |||
'etelemetry' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add trailing ,
right away to minimize next diff ;)
heudiconv/cli/run.py
Outdated
print("Could not check for version updates: ", e) | ||
else: | ||
if latest and 'version' in latest: | ||
print("Your version: {0} Latest version: {1}".format(__version__, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also I wonder if it should use lgr.info
(or .warning
) instead of a pure print
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another thing to consider is CI usage skewing our metrics, I've created an issue at mgxd/etelemetry-client#1.
* upstream/master: Update heudiconv/info.py PIN: Track effigies/fix/heudiconv_issues PIN: Nipype master
Codecov Report
@@ Coverage Diff @@
## master #369 +/- ##
==========================================
- Coverage 74.29% 74.24% -0.06%
==========================================
Files 35 35
Lines 2708 2714 +6
==========================================
+ Hits 2012 2015 +3
- Misses 696 699 +3
Continue to review full report at Codecov.
|
@mgxd and @yarikoptic - could we merge this and cut a release? |
Let's merge Have you tried it "in a wild"? (would be nice to have some testing done before releasing) |
a few other packages are sending signals. we need to make an interface to summarize the signals for the developer. |
This is largely a bug fix. Metadata and order of `_key-value` fields in BIDS could change from the result of converting using previous versions, thus minor version boost. 14 people contributed to this release -- thanks [everyone](https://github.com/nipy/heudiconv/graphs/contributors)! Enhancement - Use [etelemetry](https://pypi.org/project/etelemetry) to inform about most recent available version of heudiconv. Please set `NO_ET` environment variable if you want to disable it ([#369][]) - BIDS: - `--bids` flag became an option. It can (optionally) accept `notop` value to avoid creation of top level files (`CHANGES`, `dataset_description.json`, etc) as a workaround during parallel execution to avoid race conditions etc. ([#344][]) - Generate basic `.json` files with descriptions of the fields for `participants.tsv` and `_scans.tsv` files ([#376][]) - Use `filelock` while writing top level files. Use `HEUDICONV_FILELOCK_TIMEOUT` environment to change the default timeout value ([#348][]) - `_PDT2` was added as a suffix for multi-echo (really "multi-modal") sequences ([#345][]) - Calls to `dcm2niix` would include full output path to make it easier to discern in the logs what file it is working on ([#351][]) - With recent [datalad]() (>= 0.10), created DataLad dataset will use `--fake-dates` functionality of DataLad to not leak data conversion dates, which might be close to actual data acquisition/patient visit ([#352][]) - Support multi-echo EPI `_phase` data ([#373][] fixes [#368][]) - Log location of a bad .json file to ease troubleshooting ([#379][]) - Add basic pypi classifiers for the package ([#380][]) Fixed - Sorting `_scans.tsv` files lacking valid dates field should not cause a crash ([#337][]) - Multi-echo files detection based number of echos ([#339][]) - BIDS - Use `EchoTimes` from the associated multi-echo files if `EchoNumber` tag is missing ([#366][] fixes [#347][]) - Tolerate empty ContentTime and/or ContentDate in DICOMs ([#372][]) and place "n/a" if value is missing ([#390][]) - Do not crash and store original .json file is "JSON pretification" fails ([#342][]) - ReproIn heuristic - tollerate WIP prefix on Philips scanners ([#343][]) - allow for use of `(...)` instead of `{...}` since `{}` are not allowed ([#343][]) - Support pipolar fieldmaps by providing them with `_epi` not `_magnitude`. "Loose" BIDS `_key-value` pairs might come now after `_dir-` even if they came first before ([#358][] fixes [#357][]) - All heuristics saved under `.heudiconv/` under `heuristic.py` name, to avoid discrepancy during reconversion ([#354][] fixes [#353][]) - Do not crash (with TypeError) while trying to sort absent file list ([#360][]) - heudiconv requires nipype >= 1.0.0 ([#364][]) and blacklists `1.2.[12]` ([#375][]) * tag 'v0.6.0': (60 commits) Version boost to 0.6.0 DOC: populate detailed changelog for 0.6.0 and tune up formatting in previous one Fix miscellaneous typos in ReproIn heuristic file. BF: fix check for the sbatch (SLURM) not being available ENH: make test-compare-two-versions take any two worktrees, and just show diff if results already known Update heudiconv/convert.py apply @mgxd 's suggestions, adding a warning and a timeout environment variable need str typecast Use empty string not None Empty acq_time results in empty cell not 'n/a' DOC: Clarify tarball session handling remove repetitive import statement respond to review - add explicit py2 check - change file saving strategy - use logger instead of print fix remaning py2 errors MNT: Add Python support metadata to package fix some python2/3 incompatibilities add return data (accidently removed return) make content unicode test that load_json provides filename if invalid explicitly name invalid json ...
[0.6.0] - 2019-12-16 This is largely a bug fix. Metadata and order of `_key-value` fields in BIDS could change from the result of converting using previous versions, thus minor version boost. 14 people contributed to this release -- thanks [everyone](https://github.com/nipy/heudiconv/graphs/contributors)! Enhancement - Use [etelemetry](https://pypi.org/project/etelemetry) to inform about most recent available version of heudiconv. Please set `NO_ET` environment variable if you want to disable it ([#369][]) - BIDS: - `--bids` flag became an option. It can (optionally) accept `notop` value to avoid creation of top level files (`CHANGES`, `dataset_description.json`, etc) as a workaround during parallel execution to avoid race conditions etc. ([#344][]) - Generate basic `.json` files with descriptions of the fields for `participants.tsv` and `_scans.tsv` files ([#376][]) - Use `filelock` while writing top level files. Use `HEUDICONV_FILELOCK_TIMEOUT` environment to change the default timeout value ([#348][]) - `_PDT2` was added as a suffix for multi-echo (really "multi-modal") sequences ([#345][]) - Calls to `dcm2niix` would include full output path to make it easier to discern in the logs what file it is working on ([#351][]) - With recent [datalad]() (>= 0.10), created DataLad dataset will use `--fake-dates` functionality of DataLad to not leak data conversion dates, which might be close to actual data acquisition/patient visit ([#352][]) - Support multi-echo EPI `_phase` data ([#373][] fixes [#368][]) - Log location of a bad .json file to ease troubleshooting ([#379][]) - Add basic pypi classifiers for the package ([#380][]) Fixed - Sorting `_scans.tsv` files lacking valid dates field should not cause a crash ([#337][]) - Multi-echo files detection based number of echos ([#339][]) - BIDS - Use `EchoTimes` from the associated multi-echo files if `EchoNumber` tag is missing ([#366][] fixes [#347][]) - Tolerate empty ContentTime and/or ContentDate in DICOMs ([#372][]) and place "n/a" if value is missing ([#390][]) - Do not crash and store original .json file is "JSON pretification" fails ([#342][]) - ReproIn heuristic - tolerate WIP prefix on Philips scanners ([#343][]) - allow for use of `(...)` instead of `{...}` since `{}` are not allowed ([#343][]) - Support pipolar fieldmaps by providing them with `_epi` not `_magnitude`. "Loose" BIDS `_key-value` pairs might come now after `_dir-` even if they came first before ([#358][] fixes [#357][]) - All heuristics saved under `.heudiconv/` under `heuristic.py` name, to avoid discrepancy during reconversion ([#354][] fixes [#353][]) - Do not crash (with TypeError) while trying to sort absent file list ([#360][]) - heudiconv requires nipype >= 1.0.0 ([#364][]) and blacklists `1.2.[12]` ([#375][]) * tag 'v0.6.0': Boost perspective release date in changelog to today ENH(TST): Fix version to older pytest to ease backward compatibility testing RF: use tmpdir not tmp_path fixture FIX: minor typo in CHANGELOG.md
No description provided.