sunportal
sunportal copied to clipboard
Several changes and new features
This pull request may not be finished yet, so @philipptrenz please add your comments or contribute (in a new branch would be best).
Overview of changes in this pull request:
- added a total chart (based on year chart calculations)
- fixed capitalization for displayed strings
- improved labels of data
- walk through days (with existent data!) by clicking on bars
- unified calculations of sums (SQL sums and python sums are unequal!)
- optimized loading times by only refreshing month and day data (we may think about that?)
- updated README.md according to these changes (we may mention more of these)
When some changes are missing, I'll add these to the changelog.
Hey @nfsprodriver, many thanks for your contribution! Just tested it, it adds a lot of features I also wanted and it works great. Which additional features do you plan to implement?
Moved your changes into nfsprodriver-dev
branch
As pylint failed, I took a look into util/database.py
and I understand what you tried to achieve. Nevertheless, from my point of view the backend processing should be stateless, as there can be more than one client connected and the API should be REST-like.
So the frontend should specify the data it wants to update. This could be done within the JSON like so:
static/js/main.js
, line 449:
var request_data = {
date: currentDay
requested_data: {
day = true,
month = true,
year = false,
years = false,
total = false
}
};
Then the backend builds the response JSON only with the requested data.
OR
My preferred solution would be to have several API endpoints for the different data entities. So that the frontend no longer requests to /update
, but instead to
GET /day
GET /month
GET /year
GET /total
For initial loading there could be a GET /all
endpoint and afterwards only the necessary updates get requested (if needed with multiple queries).
Thanks @philipptrenz for the fast answer. For now, I don't have any idea for more features, which are necessary in the PR, so I'd like to feature freeze it. Talking about the update thing your point is very good to move it out of the backend. I don't know much about REST-API than it may need some work. So for this PR I'll try to implement your JSON proposal. If there's anything else, which needs to be changed before merging, please tell me ;)
Did you mean like this 8dca723c5a9282391fa80075ca79e6321aa3ce57 ?
One little thing: In the total chart the first year is cut off a little bit. May we add the year before first data on X-axis or do you have a better idea?
Found the following issues:
- Pulled your latest changes and found, that your changes increases the loading time by a factor of 30. The current master branch loads within 130ms on my machine, with your changes it's around 4 seconds.
- Your Python code is not PEP8 conform, you can check that by using a linting tool like pylint
- Your global variables in
util/database.py
do not fit the object-oriented style of this class
One little thing: In the total chart the first year is cut off a little bit. May we add the year before first data on X-axis or do you have a better idea?
Yeah, this can be changed by manipulating the from and to values of the chart, like within the following lines:
// update scale
all.tot.interval.from = moment.unix(all.tot.interval.from).subtract('months', ???);
all.tot.interval.to = moment.unix(all.tot.interval.to).add('months', ???);
totChart.data.labels = [
all.tot.interval.from,
all.tot.interval.to
];
About the conformidity, pylint does not only detects my lines as unconfirm. Can you explain, what you mean? I'm very new to this topic.
This is a proposal, which you could have meant https://github.com/philipptrenz/sunportal/pull/2/commits/a6b54ddfc47b1196f00f886fbabed4dd8ae6fed6