search

imr-framework / pypulseq

Pulseq in Python
https://pypulseq.readthedocs.io
GNU Affero General Public License v3.0
118 stars 63 forks source link

JOSS Review: Sequence version out of sync with release version. #21

Closed mathieuboudreau closed 5 years ago

mathieuboudreau commented 5 years ago

Current release version is 1.2.2 (as seen here)

...but the generated pulse sequence files report version 1.2.1, because of the hardcoded values here:

https://github.com/imr-framework/pypulseq/blob/2268bf670e68c59404c1c322f2ce12b0e00c7439/pypulseq/Sequence/sequence.py#L20-L24

So you might want to (1) fix this release sync issue, and (2) find a better way to update this value. Another point is that since you are always pushing directly to master, people area likely to have the same release version but different versions of the code, if they clone from github.

At a bare minimum, you might want to block devs from pushing directly to master (there's a setting on github to do that), and only develop on branches, where you can then have guidelines to pull request reviewers to check this value whenever reviewing pull requests. But ideally, you would have this updated in an automated way, but I don't know if that's possible.

sravan953 commented 5 years ago

Thank you for pointing this out. However, PyPulseq's versioning for the *.seq files are meant to match Pulseq's Github release to maintain compatibility across Python/MATLAB. That said, the self.version_revision should be 0, since we targeted to match Pulseq 1.2.0. :)

mathieuboudreau commented 5 years ago

Ah ha, that also makes sense. Thanks for clarifying!

sravan953 commented 5 years ago

Will fix the self.version_revision in a commit and close this issue, would that work? Will also specifically mention this in the README.

mathieuboudreau commented 5 years ago

Yup!