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

Skip dim>=3 data when reading CDF files #5975

Merged
merged 3 commits into from
Apr 1, 2022
Merged

Skip dim>=3 data when reading CDF files #5975

merged 3 commits into from
Apr 1, 2022

Conversation

jgieseler
Copy link
Member

PR Description

Fixes #5869. When reading CDF files, now variables with more than 2 dimensions (respectively dependencies) are skipped and a user warning is provided. This allows to at least read the 1- and 2-dimensional data of the corresponding CDF file (instead of just crashing). This is needed e.g. for charged particle data obtained by Parker Solar Probe's ISOIS instrument.

PR Checklist

  • I have followed the guidelines in the Contributing document
  • Changes follow the coding style of this project
  • Changes have been formatted and linted
  • Changes pass pytest style unit tests (and pytest passes).
  • Changes include any required corresponding changes to the documentation
  • Changes have been rebased and squashed to the minimal number (typically 1) of relevant commits
  • Changes have a descriptive commit message with a short title
  • I have added a Fixes #XXXX - or Closes #XXXX - comment to auto-close the issue that your PR addresses

sunpy/io/cdf.py Outdated Show resolved Hide resolved
sunpy/io/cdf.py Outdated Show resolved Hide resolved
Co-authored-by: Nabil Freij <nabil.freij@gmail.com>
@nabobalis nabobalis requested a review from dstansby March 21, 2022 21:24
@nabobalis
Copy link
Contributor

I feel like this is worth a changelog?

@nabobalis nabobalis removed the request for review from a team March 22, 2022 21:05
Copy link
Member

@dstansby dstansby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this should get a changelog entry.

@jgieseler
Copy link
Member Author

Yes, this should get a changelog entry.

Do you need me to do something for that? 🤔

@nabobalis
Copy link
Contributor

Yes, this should get a changelog entry.

Do you need me to do something for that? 🤔

It is something you can do (following: https://github.com/sunpy/sunpy/blob/main/changelog/README.rst) otherwise we can do it for you.

@jgieseler
Copy link
Member Author

Okay, I added changelog/5975.bugfix.rst to this PR:

Fixed reading CDF files when a variable has more than 2 dimensions. If this is the case the variable will be ignored, and a user warning is provided.

@nabobalis nabobalis requested a review from dstansby April 1, 2022 16:31
@dstansby dstansby merged commit 04e8645 into sunpy:main Apr 1, 2022
@sunpy-backport
Copy link

sunpy-backport bot commented Apr 1, 2022

The backport to 3.1 failed:

Commits ["9830d2bfa6604318aa06c81ad9035148d089cb4a","ec61749cf12865c1b2e7749bdd367e7a4450314d","9eb01ce9eea43512cc928b17c6d79ac06dce0ece"] could not be cherry-picked on top of 3.1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub.
git fetch
# Create new working tree.
git worktree add .worktrees/backport 3.1
# Navigate to the new directory.
cd .worktrees/backport
# Cherry-pick all the commits of this pull request and resolve the likely conflicts.
git cherry-pick 9830d2bfa6604318aa06c81ad9035148d089cb4a ec61749cf12865c1b2e7749bdd367e7a4450314d 9eb01ce9eea43512cc928b17c6d79ac06dce0ece
# Create a new branch with these backported commits.
git checkout -b backport-5975-to-3.1
# Push it to GitHub.
git push --set-upstream origin backport-5975-to-3.1
# Go back to the original working tree.
cd ../..
# Delete the working tree.
git worktree remove .worktrees/backport

Then, create a pull request where the base branch is 3.1 and the compare/head branch is backport-5975-to-3.1.

@nabobalis nabobalis added the Still Needs Manual Backport This PR needs manually backporting label Apr 1, 2022
@dstansby dstansby mentioned this pull request Apr 5, 2022
@nabobalis nabobalis removed the Still Needs Manual Backport This PR needs manually backporting label Apr 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
io Affects the io submodule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PSP/ISOIS CDF data via CDAWeb: Download works, but to_dataframe() fails
3 participants