dpinney / omf

The Open Modeling Framework for smart grid cost-benefit analysis.
https://omf.coop
GNU General Public License v2.0
112 stars 60 forks source link

BeautifulSoup pre-requisite in weather.py breaks omf import. #274

Closed dpinney closed 9 years ago

dpinney commented 10 years ago

The following throws an exception now:

import omf

The issue was BeautifulSoup was added to weather.py but wasn't added to the requirements.

Options to resolve this:

  1. The fastest way would be to remove BeautifulSoup and rewrite the code to use existing pre-reqs from requirements.txt.
  2. Slower but more complete would be to add BeautifulSoup to requirements.txt, rebuild the .zip package using these instructions, and re-test everything here
nikhilgupta10 commented 10 years ago

I will fix this issue. I am a little tempted to go with the second option (for now).

dpinney commented 10 years ago

Your call. Having another person who knows the packaging process would be good.

What we really need is a good way to run acceptance tests periodically and make sure they catch this type of issue (new pre-reqs). That was logged as #245.

nikhilgupta10 commented 10 years ago

went with the second option. "import omf" works now. So, in my GitHub>omf>: I see: 1. build; 2. dist; 3. omf (all files in this looks same as the old omf folder). What do I do now?

Another question: After I installed and opened http://127.0.0.1:5000/ works perfect. Then I tried: "python setup.py uninstall" but got an "invalid command uninstall" error.

cshjin commented 10 years ago

If you followed the installation instruction, you can try to uninstall using python setup.py develop --uninstall. If that doesn't work, try to update your setuptools package version if necessary.

nikhilgupta10 commented 10 years ago

ok, the above command works. The instructions says "python setup.py uninstall". I don't know if that was a typo, should that instruction be corrected to "python setup.py develop --uninstall"?

cshjin commented 10 years ago

Since currently it was installed in develop mode, you need to uninstall using python setup.py develop --uninstall. If it is installed in normal mode, which use python setup.py install, then python setup.py uninstall should work. python setup.py develop --> python setup.py develop --uninstall python setup.py install --> python setup.py uninstall (Cause problems at this moment)

nikhilgupta10 commented 10 years ago

alright sounds great.

nikhilgupta10 commented 10 years ago

@dpinney I tested the install on my system from the zip file.

dpinney commented 10 years ago

Sri, what was your full test process? It's not clear to me from this thread what exactly you tested.

nikhilgupta10 commented 10 years ago

@dpinney This is what I did so far: First I unzipped the omf-0.1, then from that folder, I ran "python setup.py develop", then to test the OMF:

python

import omf

and I ran it using command "python web.py", then I went to "http://127.0.0.1.5000/", page opened. Then I unistalled using command "python setup.py develop --uninstall".

dpinney commented 10 years ago

Which zip: the one you built or the one currently in git? Did you uninstall before you did the steps described above?

nikhilgupta10 commented 10 years ago

(I ran it from the one I built, NOT from the one currently in git. Sorry about the confusion)

Before I did this all: From my folder, I ran "python setup.py develop --uninstall" (python setup.py uninstall generated error). Then I edited "requirements.txt" to include Beautifulsoup. Then did the following:

  1. python setup.py sdist
  2. python setup.py bdist_wininst
  3. A new "dist" folder is created and that has a new "omf-0.1.zip".
  4. Copy-pasted omf-0.1.zip to my Downloads folder (This might not be necessary at all)
  5. Unzipped it, entered "omf-0.1"
  6. python setup.py develop
  7. python, then import omf (no error)
  8. entered omf folder, then "python web.py"
  9. went to the link, it opened.
  10. then unistalled "python setup.py develop --uninstall"
dpinney commented 10 years ago

Ah, I understand now. That's a great test process. Would have been better to use a clean machine or VM (since BeautifulSoup wasn't uninstalled), but I think what you did is thorough. Feel free to commit your new .zip to the git repo (replacing the old one).

cshjin commented 10 years ago

One thing I would like to mention, but may not be related to this issue. To my experience, the function _downloadWeather can be optimized by changing address = "http://www.wunderground.com/history/airport/{}/{:d}/{:d}/{:d}/DailyHistory.html?req_city=NA&reqstate=NA&req_statename=NA&format=1".format(airport, year, month, day) into address = "http://www.wunderground.com/history/airport/{}/{:d}/{:d}/{:d}/DailyHistory.html?format=1".format(airport, year, month, day) And also to remove the tag <br> at the end of each line. I have the experience of downloading 20 years hourly historical weather data using the later one in about 30 minutes.

nikhilgupta10 commented 10 years ago

@dpinney committed new omf-0.1.zip file. It is available now.

dpinney commented 10 years ago

Sri, can you implement @jinhw1989's suggestion above? Then we can close this issue.

nikhilgupta10 commented 10 years ago

@jinhw1989 I will check on weather.py following your suggestion.

nikhilgupta10 commented 10 years ago

@jinhw1989 I changed the "WU" link and checked with two airports (LAX, AJO)...works good. Checkedin the new code. Some other minor changes include: Proper Time calculation. Its in a much better shape now.

Although I have a small question, what do you mean by: "And also to remove the tag
at the end of each line."

cshjin commented 10 years ago

I knew at the retrieved .csv file, the last column has a <br> tag there. I thought that is not a big deal, if you don't want to use the DateUTC data. @dpinney The way I think about using DateUTC because it doesn't have daylight saving time, we encountered this kind of issue several times such as import scada data or somewhere.

dpinney commented 10 years ago

Eventually we need to decide whether we (1) do everything in UTC or (2) attempt to support time zones.

jcfuller1 commented 10 years ago

Time zones can be pretty important, especially if you are looking at energy conservation / impacts. While its a pain in the to use, studies show that they impact energy consumption by as much as 0.5-1.0% per day of daylight savings. Not using DST will also shift your peak by one hour during DST months, which is now >7 months of the year.

dpinney commented 10 years ago

:cry:

dpinney commented 9 years ago

Is this issues fixed? If so, let's close it.

nikhilgupta10 commented 9 years ago

yes. I will checkin weather.py today. you can close this issue.

dpinney commented 9 years ago

Please close this issue when you commit the finished code.

dpinney commented 9 years ago

What's this .exe? image

nikhilgupta10 commented 9 years ago

I actually don't know. Just saw that... I think that needs to be deleted (I thought I did not commit that one). Its not related to the omf-0.1.zip

dpinney commented 9 years ago

You checked in the .exe but you didn't check in any .zip file.

So where is the zip you made and then tested?

nikhilgupta10 commented 9 years ago

@dpinney can please check the 'dist' folder now? (sorry about that...)

dpinney commented 9 years ago

Thanks! I did a quick check of the zip and the contents look good. Re-closed.