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

Key results for detail panel #65

Merged
merged 26 commits into from
Apr 23, 2024
Merged

Conversation

henhuy
Copy link
Contributor

@henhuy henhuy commented Mar 28, 2024

Closes #52

Waits for ionrangeslider fix or shall I do it using AJAX?

@henhuy henhuy requested a review from nesnoj March 28, 2024 13:06
@henhuy henhuy self-assigned this Mar 28, 2024
Copy link
Contributor

@nesnoj nesnoj left a comment

Choose a reason for hiding this comment

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

Thx for this feature!
If you think it's worth to wait for the ion fix (and it's expected to come), we can give it a couple of more days. But if this solution is ok for you and the other wouldn't be much slicker, go for it.
BTW, currently the app doesn't start at my end:

Traceback (most recent call last):
  File "/app/manage.py", line 32, in <module>
    execute_from_command_line(sys.argv)
  File "/venv/lib/python3.9/site-packages/django/core/management/__init__.py", line 442, in execute_from_command_line
    utility.execute()
  File "/venv/lib/python3.9/site-packages/django/core/management/__init__.py", line 416, in execute
    django.setup()
  File "/venv/lib/python3.9/site-packages/django/__init__.py", line 24, in setup
    apps.populate(settings.INSTALLED_APPS)
  File "/venv/lib/python3.9/site-packages/django/apps/registry.py", line 124, in populate
    app_config.ready()
  File "/app/digiplan/map/apps.py", line 21, in ready
    from digiplan.map import hooks as digiplan_hooks
  File "/app/digiplan/map/hooks.py", line 10, in <module>
    from digiplan.map import config, datapackage, forms
  File "/app/digiplan/map/forms.py", line 65, in <module>
    class PanelForm(TemplateExtraContentForm):  # noqa: D101
  File "/app/digiplan/map/forms.py", line 91, in PanelForm
    def generate_fields(self, parameters: dict, additional_parameters: dict | None = None) -> dict:
TypeError: unsupported operand type(s) for |: 'type' and 'NoneType'

I merged the dev which included #59 but this shouldn't be the problem?!

@nesnoj
Copy link
Contributor

nesnoj commented Apr 3, 2024

Or maybe it's simply not finished? :D

@henhuy
Copy link
Contributor Author

henhuy commented Apr 3, 2024

This is due to your python version - seems outdated.
I will update python version in dockerfile in separate PR

@henhuy
Copy link
Contributor Author

henhuy commented Apr 3, 2024

Fixed it with future annotation

@nesnoj
Copy link
Contributor

nesnoj commented Apr 3, 2024

Fixed it with future annotation

Great, error vanished.
However, the values do not change on slider adjustments but I guess this is the part you will integrate as soon as we decide for ion/AJAX way?

@nesnoj
Copy link
Contributor

nesnoj commented Apr 9, 2024

Ok, as there's no progress at ion, please implement using ajax.

@henhuy henhuy force-pushed the feature/key-results-for-detail-panel branch from ff75e57 to 6418264 Compare April 10, 2024 08:00
@henhuy henhuy marked this pull request as ready for review April 10, 2024 09:17
@henhuy
Copy link
Contributor Author

henhuy commented Apr 10, 2024

Implemented AJAX calls.
Had to change slider events from "onChanging" to "onFinish", as sliders fired too often.
Now they only fire, once user lifts mouse.
Please check if this is okay...

Copy link
Contributor

@nesnoj nesnoj left a comment

Choose a reason for hiding this comment

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

I rebuilt the docker but I'm getting ModuleNotFoundError: No module named 'template_partials' here..

@@ -1175,6 +1189,24 @@ files = [
Django = ">=2.2"
redis = ">=3.0.0"

[[package]]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Strange? Cause django-template-partials is added in poetry.locks...
Can't explain right now

@nesnoj nesnoj self-requested a review April 18, 2024 13:50
Copy link
Contributor

@nesnoj nesnoj left a comment

Choose a reason for hiding this comment

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

Hey @henhuy, great feature nicely integrated!
I fixed the calculations, values are correct now. I have 2 questions:

  • wind_2024 (s_w_6): the slider value must be divided by its max value, I did this in bb090f8. Is it ok this way or should the max value rather be obtained from context ?
  • PV roof potential: you used the capacity, however, in the current implementation areas must be used. I changed this in ede4085. Unfortunately in the pipeline I had used sqm not sqkm like for the other technologies. Therefore I changed the pipeline to get everything in line. You'll need the latest potentialarea_pv_roof_area_stats_muns.csv from the wolke to make it work.

If you're fine with my changes, please go ahead and merge..

PS: BTW, the fct calculate_potential_shares seems to be outdated..

@henhuy
Copy link
Contributor Author

henhuy commented Apr 23, 2024

Your fixes are working. Reading max wind from energy settings panel is totally fine.
I moved adaption of potential shares calculation to new issue #91
You can merge now

@henhuy henhuy requested a review from nesnoj April 23, 2024 07:36
@nesnoj nesnoj merged commit 4620d50 into dev Apr 23, 2024
1 check passed
@nesnoj nesnoj deleted the feature/key-results-for-detail-panel branch April 23, 2024 08:04
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.

Calculate key results for detail settings
2 participants