Skip to content

fid_proc support and various bug fixes#32

Merged
headmeister merged 1 commit intoisi-nmr:masterfrom
VKarlaftis:master
Feb 24, 2026
Merged

fid_proc support and various bug fixes#32
headmeister merged 1 commit intoisi-nmr:masterfrom
VKarlaftis:master

Conversation

@VKarlaftis
Copy link
Contributor

@VKarlaftis VKarlaftis commented Feb 12, 2026

  • Expanded support to fid_proc.64 files ('fid_proc' differs from 'fid' in regards to where the file is stored relative to the parameter_files, see RELATIVE_PATHS code)
  • Bug fix on deleting non-existent attributes in unload_properties
  • Bug fix on _validate method as all parameter_files were expected inside the parent folder instead of the specific paths defined in RELATIVE_PATHS.
  • Bug fix on dwell_s when sw_hz is zero

@wtclarke
Copy link
Contributor

Hi @headmeister,

We'd love to get this PR integrated so that a downstream tool spec2nii can be updated to handle new PV360 versions. Any chance that you could take a look over the next few days?

@headmeister
Copy link
Contributor

headmeister commented Feb 24, 2026

Hi, sorry @wtclarke I should somehow enable notifications for new pull requests... In any case, If I don't respond timely tag me as you did now so that I won't miss new updates.

Anyways, I may need a bit more context. This is related to spectroscopic datasets only right? Since I have not seen these fid.64s fid_procs anywhere else. We should likely add tests for reading these as well later down the line then.

But for now, I'd like to ask just to tidy up the commits a bit please. So if you could squash those that e.g. disable/reenable the file size checks would be nice. I believe this all might just fit into a single commit in the end.

And ideally please fix the formatting issue, as with 2dseq just spread the fid_proc schema over multiple lines.

Thanks for your contribution and once this is resolved I will approve this and create a new release.
Best regards,
Jiri

@wtclarke
Copy link
Contributor

Hi @headmeister , Thanks for the quick and positive response. @VKarlaftis will look into squashing the commits together and the formatting issue. I think he also has a source of official Bruker test data which hopefully has some of the new fid format data.

@VKarlaftis VKarlaftis force-pushed the master branch 2 times, most recently from 25cbdb1 to d0cf6b5 Compare February 24, 2026 13:34
@VKarlaftis
Copy link
Contributor Author

Hi @headmeister. I've now squashed the commits.

Re fid_proc.64 files. Yes, as I understand it, this is a Spectroscopy format and there is one available in the following repo:
https://github.com/cecilyen/PV360_StdData/tree/main/PRESS_1H/pdata/1

PS: This is the repo shared here: #25

@headmeister
Copy link
Contributor

Hi @VKarlaftis, ok thank you for squashing the commits, I see now. Its some form of processed fids, likely with phase correction... ok. We will then later also likely need the fid_refscan.64 won't we? I don't see a reason why this should block this PR though, just for future reference.

Jiri.

@headmeister headmeister merged commit fa1a38e into isi-nmr:master Feb 24, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants