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

Panel Data #36

Merged
merged 14 commits into from
Feb 19, 2024
Merged

Panel Data #36

merged 14 commits into from
Feb 19, 2024

Conversation

jinayang15
Copy link
Contributor

Uploads solar panel data to PostgreSQL database

Python script to upload panel data to PostgreSQL database
Added README and a couple comments
Copy link
Member

@probro27 probro27 left a comment

Choose a reason for hiding this comment

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

@jinayang15 Great job getting this working. I had a couple of comments on this regarding using SQLAlchemy and a more abstract code structure with functions and classes.
If you feel it is too much of a hassle then let me know, but I feel it could be a good structure to work with.
Also please fix the linting issues. Most of them can be fixed by running black panels-data.py (installation here: https://github.com/psf/black?tab=readme-ov-file#installation-and-usage)
Thank you!

scripts/panels_data/panels_data.py Outdated Show resolved Hide resolved
scripts/panels_data/README.txt Outdated Show resolved Hide resolved
scripts/panels_data/README.txt Outdated Show resolved Hide resolved
- Changed to work as a function and abstracted the queries to allow Solar Panel to work as a class
@jinayang15 jinayang15 requested a review from probro27 February 3, 2024 03:58
Copy link
Member

@probro27 probro27 left a comment

Choose a reason for hiding this comment

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

@jinayang15 Thank you for taking care of this. I have mentioned a couple of things you need to change. Further, remove the old panels_data.py file and move the requirements.txt and README to the same folder as the new panels_data.py file. Also if you have the database credentials, try adding it and giving us a photo of the data being there. If you don't have let us know and we can check that out.

Also please fix the linting issues using python-black and for further details check the logs of the job that is failing. Let me know if you face any trouble and I can help you out with that.

scripts/panels_data/README.txt Outdated Show resolved Hide resolved
panels_data.py Outdated Show resolved Hide resolved
panels_data.py Outdated Show resolved Hide resolved
scripts/panels_data/panels_data.py Outdated Show resolved Hide resolved
@jinayang15
Copy link
Contributor Author

jinayang15 commented Feb 3, 2024

Screenshot 2024-02-03 at 12 03 57 PM

This is what the data looks like in the database. I think the linting issues should be fixed? If there are still problems with that, I may need some help.

@jinayang15 jinayang15 requested a review from probro27 February 3, 2024 17:06
Copy link
Member

@probro27 probro27 left a comment

Choose a reason for hiding this comment

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

@jinayang15 From our previous discussion with Forest, please use the brochure linked in the confluence page to access the dimensions and other data and add that in as well. Also please do not push .DS_Store files to the repository. Thanks :)

@jinayang15
Copy link
Contributor Author

@jinayang15 From our previous discussion with Forest, please use the brochure linked in the confluence page to access the dimensions and other data and add that in as well. Also please do not push .DS_Store files to the repository. Thanks :)

Just to clarify, what specific data would you like me to add? Do we want width and height or just area? Also, is there any other information that would be useful?

@probro27
Copy link
Member

@jinayang15 You can push height, width and surface area. More data wouldn't hurt. That should be good. Thanks!

@jinayang15
Copy link
Contributor Author

image This is what the table looks like now. Let me know if there is anything else I should change!

Copy link
Member

@probro27 probro27 left a comment

Choose a reason for hiding this comment

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

@jinayang15 LGTM! The data looks good, thanks for pushing this and checking out all of my concerns.

If the workflow passes, we can merge the PR in.

Copy link
Member

@probro27 probro27 left a comment

Choose a reason for hiding this comment

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

@jinayang15 I think you need to fix some linting issues, so please do that first! Think they would be fixed by running the black formatter, but if you are unable to figure out, let me know and I can help you out.

@jinayang15
Copy link
Contributor Author

@jinayang15 I think you need to fix some linting issues, so please do that first! Think they would be fixed by running the black formatter, but if you are unable to figure out, let me know and I can help you out.

Shoot, I forgot about it again. Should be good now!

Copy link
Member

@probro27 probro27 left a comment

Choose a reason for hiding this comment

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

@jinayang15 We can merge the PR in now

@jinayang15 I think you need to fix some linting issues, so please do that first! Think they would be fixed by running the black formatter, but if you are unable to figure out, let me know and I can help you out.

Shoot, I forgot about it again. Should be good now!

Don't worry its all good!

@probro27 probro27 merged commit 8f81757 into uw-midsun:main Feb 19, 2024
2 checks passed
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.

2 participants