search

SEED-platform / seed

Standard Energy Efficiency Data (SEED) Platform™ is a web-based application that helps organizations easily manage data on the energy performance of large groups of buildings.
Other
107 stars 55 forks source link

Error on importing xlsx with empty first sheet #2634

Closed macintoshpie closed 3 years ago

macintoshpie commented 3 years ago

Describe the bug Importing xlsx file with empty first sheet results in error

Expected Behavior It should not error if data from other sheets will be used

Steps to Reproduce

  1. upload PM meter example file (data_imports/tests/data/example-pm-monthly-meter-usage.xlsx)
  2. It will fail claiming it's empty
macintoshpie commented 3 years ago

Suggested fix: on this line, set sheet_name=None to read all sheets (defaults to first) before checking if any sheets have data https://github.com/SEED-platform/seed/blob/3547ce6e9012fcbb13e477ad5fbc6b0c2f648c37/seed/views/v3/uploads.py#L105 ref: https://pandas.pydata.org/docs/reference/api/pandas.read_excel.html

RDmitchell commented 3 years ago

Instance; Dev1 SHA: b12fbd00f

@adrian-lara / @macintoshpie -- does this only apply to PM Meter spreadsheets?

I tried uploading a normal ESPM spreadsheet that just has one tab, added a blank tab as the first tab, tried importing it, and get a 503 error. But maybe that's not the test I should be doing. image

RDmitchell commented 3 years ago

Instance; Dev1 SHA: b12fbd0

Loading the file "example-pm-monthly-meter-usage.xlsx" from the repo which does have a blank first tab, it seems like the program imported it image

But now it appears "stuck" here image

RDmitchell commented 3 years ago

Instance; Dev1 SHA: b12fbd0 LBNL Test 200 2015 Compliance Cycle

I tried the import again, and this time, it got past the progress bar and showed the 2nd screen indicating that it had imported the records image

The Inventory Property tab is empty, I guess because there were no records to match to? image

The file has the PM Property IDs, so I was thinking that SEED could make those parent records and show them in the Inventory list, and then link to the meter data?

But that doesn't appear to be the case ... ??

RDmitchell commented 3 years ago

@adrian-lara -- see the previous comment to figure out if my assumption about making the parent records is correct or not.

macintoshpie commented 3 years ago

@RDmitchell This should only be successful when importing meter data. ESPM defaults to first sheet when importing, so maybe we should add an additional check to discriminate between meter/ESPM. I would consider this sufficient to merge for now.

I've created this ticket to handle empty first sheet for ESPM: #2648

RDmitchell commented 3 years ago

@macintoshpie -- except that I don't see any new records in the Inventory list -- is this because the parent records didn't already exist?

macintoshpie commented 3 years ago

@RDmitchell when you import the pm meters data it won't create inventory for you, it will only link the meter data it's importing to existing properties.

RDmitchell commented 3 years ago

@Ryoken -- hmm, ok, but it might be useful to tell the user that what they are doing is meaningless, ie, doing the import I did resulted in me not being able to see anything that I thought was imported.

@adrian-lara -- will make a new issue for this ... I actually thought that the program would make the Inventory records if the meter data had the PM Property ID, but I guess I'm wrong.

So I guess this can be closed.

RDmitchell commented 3 years ago

I just made a separate issue to address the fact that I think the program should be able to make the parent Property records if the PM Property ID is in the data, which I believe it always is for the ESPM meter data.

2672