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

Updated version of the tool #17

Merged
merged 39 commits into from
Oct 16, 2019
Merged

Updated version of the tool #17

merged 39 commits into from
Oct 16, 2019

Conversation

FedericoTartarini
Copy link
Collaborator

Main contributions:

  • Fixed GitHub issues Low velocity PMV results calculation error in Python #16, Strict input validation #3
  • Added heat loss vs air temperature chart
  • Added Upload page
  • Fixed the air velocity chart
  • Updated help files
  • Removed LEED (as suggested by Stefano)
  • Improved chart graphics
  • Added Boostrap to the webpage so now the website is responsive and mobile friendly.
  • Updated the graphics (navigation bar, footnote, charts color) using same images and colour as the official CBE website.
  • Added link to MRT tool developed by Tyler
  • Updated Google Analytics code (now we can track traffic in all pages), created a Gmail account for the website and added contact email.
  • Change list of Authors (as suggested by Stefano)
  • Add the psychrometric variables to the RH-dry-bulb chart
  • created a global.js which contains shared functions
  • Added unittest module for Python functions
  • Fixed help me
  • Moved from Python 2 to 3

For more information please look at the list of commits below. In addition, please access the update version of this tool using this link

- added psychrometric variables to the RH-dry-bulb chart
- removed white region from velocity chart
- added grid lines velocity chart
- increased x_limits velocity chart
- removed air velocity limits if clo>.7 and met>1.3
- added description in the velocity chart about applicability limit
- changed how results are displayed ASHRAE page
- changed navbar, footer and page layout
- fixed issue with adaptive ASHRAE chart showing too warm when cold
- removed css classes
- fixed minor issues
- added new code google Analytics
- restored update page
- python code now return dictionaries
- removed LEED tab
- changed list of authors
- added contact us section
- edited footer
- imporved looked of EN and ASHRAE pages
- improved layout help page
- added new icons (including favicon)
- changed colours webpage to match CBE official webpage
- changed gridlines charts
- changed colours comfort zones charts
- fixed issues with units not changing Temp-Hum chart
- re-enabled button globe temp calculation
- added new lines to heatLoss chart (met, total latent and sens)
- added gridlines to velopt chart with IP units
- added card around inputs and removed background
- update page results changed the rounding PPD
- removed LEED from Readme
- navbar shows page currently open
- fixed issue with button air speed control
- renamed charts
- Cooling effect only shows if air speed > 0.2
- added old Google Analytics code to the code
- update layout compare (there are still some known issues: SI/IP, Help file, Local air control, globe temp, CE showing)
- fixed all help files
- created new help images
- removed unecessary css files
- EN removed local control air speed
- changed message when speed outside range is selected
- Compare - removed vel range and fixed calculations
- Global - fix code when met >1.3 and clo>0.7
- aligned numbers left
- aligned columns compare left
- saved in memory user inputs solar calc
- moved solar calc button location
@chriswmackey
Copy link

@FedericoTartarini ,

I just wanted to note again that I plan to send a PR for the Python version that is compatible with both Python 2 and 3, does not require any dependencies, includes all of the documentation that you see on the ladybug comfort repo, and includes all of our unittests

If you want to keep editing the python here, that is fine but this is really starting to feel like duplicated efforts and it might just be best for you to signal to me the best time for me to send my PR (when you are not editing the code base here).

@FedericoTartarini
Copy link
Collaborator Author

Hi Chris, Tyler has to approve the current Pull Request and then you can edit the Python code. Please keep in mind that the Python code is now used by Flask to calculate the comfort parameters (PMV, CE, PPD, etc.) in the Update tab. The Python function comfPMVElevatedAirspeed() now returns a dictionary and not a list.

@chriswmackey
Copy link

Thanks for clarifying @FedericoTartarini . I'll aim to not change the output structure (I'm also using a dictionary in my newer code) or the names of the functions in any edits I make here. Also, once we have this merged, I'll take some time to try and understand how you are using the Python code on the back end with Flask.

Copy link
Contributor

@thoyt thoyt left a comment

Choose a reason for hiding this comment

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

There's quite a bit here so I can't say I dug into everything but it looks good overall! I don't think we can merge this without bringing down the deployed tool, I'd have to disable the githook first. I'm still not clear if you are able to deploy this yourself.. do you have a schedule for deploying?

comfort.py Outdated Show resolved Hide resolved
runtime.txt Outdated Show resolved Hide resolved
Procfile Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
comfort.py Outdated Show resolved Hide resolved
@FedericoTartarini
Copy link
Collaborator Author

Hi @thoyt unfortunately I do not think I will be able to deploy it myself. Ideally it would be great if we could have it up and running by Monday next week so Stefano can show it during the CBE symposium/meeting. I will try to address all your comments as soon as possible.

@FedericoTartarini
Copy link
Collaborator Author

Heroku requires the Procfile and runtime.txt files and cannot run application without them. I have currently removed them from master branch as you requested but now Stefano cannot longer access the application on Heroku which is connected to the Master branch.

@thoyt
Copy link
Contributor

thoyt commented Oct 16, 2019

Would it help to use a special branch for the heroku deployment?

@FedericoTartarini
Copy link
Collaborator Author

it would, but at the same time I would have to update the code in both branches (master and special). In addition we are planning to use Heroku only for the testing phase and for nothing else, hence, once we are done with the testing I am planning to take the Heroku app down.

Arens has messaged me since he cannot access the Heroku app anymore. How long will it take to have the new version of the CBE tool online? If it is going to take a week I will have to commit to the master branch the two files I have removed

@FedericoTartarini
Copy link
Collaborator Author

To solve the issue with Heroku, I have now stopped automatic deployments on Heroku.

@thoyt thoyt changed the base branch from master to python3 October 16, 2019 05:34
@thoyt
Copy link
Contributor

thoyt commented Oct 16, 2019

I'm going to merge this into a special branch for deployment. I can take care of the extraneous files and it shouldn't affect your heroku deployment.

@thoyt thoyt merged commit f9e25ca into CenterForTheBuiltEnvironment:python3 Oct 16, 2019
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.

3 participants