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

IndexError: list index out of range #277

Closed
linxiong100 opened this issue Feb 14, 2022 · 20 comments · Fixed by #281
Closed

IndexError: list index out of range #277

linxiong100 opened this issue Feb 14, 2022 · 20 comments · Fixed by #281
Labels
bug Something isn't working

Comments

@linxiong100
Copy link
Collaborator

linxiong100 commented Feb 14, 2022

Hi, I got 'IndexError' when I am trying to read most recent ATL08 data products using 'load' from icepys.

ds = reader.load()

  File "C:\Users\xiongl21\Miniconda3\envs\icepyx-env\lib\site-packages\icepyx\core\read.py", line 443, in load
    self._build_single_file_dataset(file, groups_list)
  File "C:\Users\xiongl21\Miniconda3\envs\icepyx-env\lib\site-packages\icepyx\core\read.py", line 560, in _build_single_file_dataset
    is2ds = Read._add_var_to_ds(
  File "C:\Users\xiongl21\Miniconda3\envs\icepyx-env\lib\site-packages\icepyx\core\read.py", line 342, in _add_var_to_ds
    grp_spec_vars = [
  File "C:\Users\xiongl21\Miniconda3\envs\icepyx-env\lib\site-packages\icepyx\core\read.py", line 343, in <listcomp>
    wanted_vars[i]
IndexError: list index out of range
@JessicaS11
Copy link
Member

Hello @linxiongecu! I'm glad you were able to get some data downloaded.

Do you know which file (or list of files) is causing this issue? Then I may be able to download the same granule to try and reproduce the issue. Could you also provide me with the output of reader.vars.wanted? Thanks!

@linxiong100
Copy link
Collaborator Author

linxiong100 commented Feb 14, 2022 via email

@JessicaS11
Copy link
Member

Thanks for providing the file and output. It allowed me to reproduce the IndexError and start debugging. It turns out there was a mismatch in list lengths that was causing the issue. I've resolved that (see this branch). Another error has arisen due to the longer path lengths of variables in ATL08 versus the datasets I'd initially tested on. I'll continue working through that error next, and we should be able to get these fixes through in the next release (I'm planning for one this week or next).

@JessicaS11
Copy link
Member

Update: the issue is that when Xarray reads in a deeper layer variable (e.g. gtXY/land_segments/canopy/variable), it brings the dimensions (here, delta_time is the one we need) in without coordinates, because the coordinates live a level higher at gtXY/land_segments/delta_time. As best I understand it, Xarray cannot read in multiple hdf5 groups at once, so the solution will be to bring the two groups into separate DataSets and merge them. Work is ongoing...

@JessicaS11 JessicaS11 linked a pull request Feb 25, 2022 that will close this issue
@JessicaS11
Copy link
Member

@linxiongecu I implemented the fix last week. @weiji14 suggested you might be a good person to review the PR (#281) so we can get it merged into the development branch. Are you able/willing to do that? If so, I will add you to the organization so you have PR review permissions.

@linxiong100
Copy link
Collaborator Author

Sure. I can review that! Thanks for adding me!

@JessicaS11
Copy link
Member

Awesome - thanks! You should have gotten an invite email.

@JessicaS11 JessicaS11 added the bug Something isn't working label Mar 4, 2022
@JessicaS11
Copy link
Member

Hello @linxiongecu! Could you please give an indication if you're still able to review the PR fixing this issue, and if so, when you anticipate you will be able to? I usually give people a few weeks, but we have an event coming up in 1.5 weeks and I'd like to have as many bug fixes released before the event as possible. Thanks!

@linxiong100
Copy link
Collaborator Author

Hi, @JessicaS11 , do you have any instructions for the reviewing process? I am new to this and not know how to do it. Thank you!

@JessicaS11
Copy link
Member

Hello @linxiongecu.

do you have any instructions for the reviewing process?

Yes and no. GitHub has some instructions for navigating the pull request and providing feedback. I'm currently working on a more complete guide (which will live in The Turing Way), but unfortunately it's not fully drafted yet. For this PR, our primary goal is to make sure that the issues you were having are fixed. Following @weiji14's suggestion, if you run pip install https://github.com/icesat2py/icepyx/archive/debug.zip locally to install icepyx from the PR branch, you should be able to run your code with no issues (make sure you've restarted your kernel and are in the environment where you installed the branch version). Then you can provide any comments/feedback/issues in the PR and request changes or accept the PR (much like a manuscript review, but way easier!).

I'm happy to help in any way I can!

I am new to this and not know how to do it.

Thanks for being willing to take this on. We greatly appreciate the contribution!

@ricardobarroslourenco
Copy link
Member

@JessicaS11 I have got into the same issue with h_canopy here (refer to discussion, and gist) .

I have went over your changes and they seem ok. Can I go over this review too?

@linxiong100
Copy link
Collaborator Author

linxiong100 commented Mar 14, 2022

Hi @JessicaS11 :
I tried: pip install https://github.com/icesat2py/icepyx/archive/debug.zip
But get a warning: WARNING: Built wheel for icepyx is invalid: Metadata 1.2 mandates PEP 440 version, but 'unknown' is not
When I call: reader.load(), there is no error but the kernel will die and need to restart.

@ricardobarroslourenco
Copy link
Member

ricardobarroslourenco commented Mar 14, 2022 via email

@weiji14
Copy link
Member

weiji14 commented Mar 14, 2022

Sorry, gave the wrong command. Try doing pip uninstall icepyx first, and then pip install git+https://github.com/icesat2py/icepyx.git@debug instead. Newer versions of pip > 21.0 makes things tricky (see pypa/pip#8368).

Oh and thanks for chipping in @ricardobarroslourenco! Feel free to review the code over at #281. See @JessicaS11's tips at #277 (comment) on how you can get started.

@linxiong100
Copy link
Collaborator Author

@weiji14 Thanks for the instruction. Installation works now.
But the kernel will still die when I use load() function. No error is reported.

@weiji14
Copy link
Member

weiji14 commented Mar 14, 2022

Cool, could you copy and paste the code that shows how you are calling load()? Just want to see what's happening.

@linxiong100
Copy link
Collaborator Author

linxiong100 commented Mar 14, 2022

import icepyx as ipx
import os

####need h canopy , lat , lon

import h5py
#read data
path_root = 'C:/Users/xiongl21/WorkingFolder/Postdoc/Icesat2/MyTutorial/Test/data'
pattern = "processed_ATL{product:2}_{datetime:%Y%m%d%H%M%S}_{rgt:4}{cycle:2}{orbitsegment:2}_{version:3}_{revision:2}.h5"
reader = ipx.Read(path_root, "ATL08", pattern) # or ipx.Read(filepath, "ATLXX") if your filenames match the default pattern

#h_canopy
#
reader.vars.append(var_list=["latitude", "longitude", "h_canopy"])
reader.vars.wanted
ds = reader.load()

@weiji14
Copy link
Member

weiji14 commented Mar 14, 2022

Hmm, so I tried running this on ATL08_20211222000622_13831301_005_01.h5 and it didn't crash, but the ATL08 dataset isn't loading either (it just hangs there). So something might be wrong with the bugfix in #281?

@JessicaS11
Copy link
Member

@ricardobarroslourenco

I have went over your changes and they seem ok. Can I go over this review too?

That would be great - thanks for the offer and getting involved in this conversation. Check your email for an org invite.

@ricardobarroslourenco
Copy link
Member

Hi @JessicaS11 and @weiji14 ! Thanks for the instructions. I have just accepted the invite and will be going over the PR review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants