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

Fix conditional header generation for NMR JCAMP export #219

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Nicolass67
Copy link

This pull request addresses an issue where certain JCAMP files, like 356_MR260_NaCp_UHV_1H.jcamp.txt, could be opened directly in NMRium but failed to load through the ELN. Specifically, the NMRium button is enabled; however, when clicked, it only displays the placeholder drag-and-drop screen instead of loading the actual data.

Context:

The root cause was identified in the __header_nmr method in chem_spectra/lib/composer/ni.py, which previously attempted to add headers unconditionally. When any data field was missing, this led to inconsistencies and failed parsing by NMRium.

Solution: This PR modifies the __header_nmr method to conditionally include each header line only if the corresponding data is present. This prevents incomplete or empty header lines that could disrupt the JCAMP file format.

Changes:

  • The method now appends header lines only if each value is non-empty.
  • A structured list (header_lines) is created by checking each data field individually.
  • Improved handling of optional header information, ensuring consistent output for each JCAMP file.

Testing:

  1. Upload the 356_MR260_NaCp_UHV_1H.jcamp file in jcamp format in the ELN.
  2. Save the dataset and ensure the ChemSpectra file is generated correctly.
  3. Confirm the NMRium button loads the data without issues.

This fix will ensure that NMRium parse the file correctly when accessed via the ELN.

Issue : data not loaded into nmrium #221

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.

2 participants