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

Remove obsolete data classes #695

Merged
merged 60 commits into from Aug 3, 2023
Merged

Remove obsolete data classes #695

merged 60 commits into from Aug 3, 2023

Conversation

Flix6x
Copy link
Contributor

@Flix6x Flix6x commented May 22, 2023

This is the big one.

  • Update conftests to set up only the new data classes (GenericAsset, Sensor and TimedBelief)
  • Update tests accordingly
  • Remove Power
  • Remove Price
  • Remove Weather
  • Remove TimedValue (on which Power, Price and Weather are based)
  • Remove Asset
  • Remove Market
  • Remove WeatherSensor
  • Remove AssetType
  • Remove MarketType
  • Remove WeatherSensorType
  • Remove schemas associated with obsolete classes (mostly in api/common/utils/validators.py; I left a few decorators with salvageable code/ideas, mainly regarding ISO8601 durations, that could be helpful in improving our Marshmallow schemas/fields)
  • Remove Resource
  • Remove query support for (already previously removed) analytics view
  • Remove query support for (already previously removed) portfolio view
  • Remove util functions dealing with obsolete classes
  • Check code for mentions of deprecations
  • Remove documentation references to obsolete classes (I couldn't find any)
  • Update documentation notes on data model transition (@nhoening probably you want to have another pass at this)
  • Sunset the fm0 scheme for entity addresses

Follow-up issues created:

  • Database migration removing the obsolete tables (worth a short discussion first on how this should be handled) Data migration after removing obsolete data classes #875 875
  • Check whether documentation section on singular/plural keys can be removed
  • Sorting RESTful index endpoints
  • Interpretation of DurationField as a timedelta by taking into account a corresponding AwareDatetimeField (see api/common/utils/validators.py)
  • Document special interpretation of generic_asset_type_name="weather station" in data.models.forecasting.model_spec_factory (it could also become a configuration setting)

Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x Flix6x self-assigned this May 22, 2023
@Flix6x Flix6x added this to In progress in Deprecate old database models via automation May 22, 2023
Signed-off-by: F.N. Claessen <felix@seita.nl>
@nhoening
Copy link
Contributor

A bit disappointing that coverage only improved by 1% :(

There is a function called new_decorator which now is uncovered. Maybe it is not used anymore?

Flix6x and others added 13 commits May 23, 2023 09:11
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
@nhoening nhoening added this to the 0.15.0 milestone Jun 15, 2023
nhoening and others added 4 commits July 11, 2023 21:14
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

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

Very satisfying to read (and also a bit nostalgic!)
Great work !

Some broader comments:

  • This PR should also have a data migration (removing structure)
  • The documentation still talks about "portfolio", "analytics" and "control" (there is even a page for each of them in the views folder, but also look at concepts/benefits_of_flex.rst and host/modes.rst and configuration.rst). There are also mentions in ui/templates/base.html

flexmeasures/api/common/schemas/sensors.py Outdated Show resolved Hide resolved
flexmeasures/api/common/utils/api_utils.py Show resolved Hide resolved
flexmeasures/conftest.py Outdated Show resolved Hide resolved
flexmeasures/conftest.py Show resolved Hide resolved
Deprecate old database models automation moved this from In progress to Review in progress Jul 17, 2023
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
@Flix6x
Copy link
Contributor Author

Flix6x commented Jul 21, 2023

The data migration is still a sizable task. Running flexmeasures db migrate generates a very large auto revision file (~200 lines) that needs to be checked manually. These predominantly deal with the downgrade.

There is also the offchance that the deleted tables contain user data that has not been migrated yet. This should only be possible in case data was added by circumventing our own classes (e.g. initializing an Asset has been leading to initialize a GenericAsset and a Sensor under the hood). However, I can't guarantee that simply removing the old tables wouldn't lead to some loss of data. Therefore, I suggest creating backups of the deleted tables.

Foreseeing this exact discussion, I had moved the database migration into the list of follow-up tasks to this PR (see task list above).

…m request variables

Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Flix6x and others added 8 commits August 1, 2023 17:50
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
…classes

# Conflicts:
#	flexmeasures/conftest.py
#	flexmeasures/data/tests/conftest.py
Signed-off-by: F.N. Claessen <felix@seita.nl>
…se template

Signed-off-by: F.N. Claessen <felix@seita.nl>
… pages

Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x Flix6x requested a review from nhoening August 1, 2023 18:42
Deprecate old database models automation moved this from Review in progress to Reviewer approved Aug 2, 2023
Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

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

Great stuff!

A thought on the data model migration: We could add a warning if we find data (e.g. >0 assets and/or sensor and/pr Power data). Didn't we have a confirmation dialogue in one of the other migrations? It would not affect users who have no such data.

Does a db upgrade call actually work on an empty database, given that the data classes are not there anymore? I believe we stopped using the classes in migrations (I did a quick check).

@Flix6x Flix6x merged commit b42f24f into main Aug 3, 2023
3 of 4 checks passed
Deprecate old database models automation moved this from Reviewer approved to Done Aug 3, 2023
@Flix6x Flix6x deleted the remove-obsolete-data-classes branch August 3, 2023 09:10
Flix6x added a commit that referenced this pull request Aug 3, 2023
Signed-off-by: F.N. Claessen <felix@seita.nl>
Flix6x added a commit that referenced this pull request Aug 3, 2023
…ces) (#770)

Add binary constraint to avoid energy leakages during periods with negative prices.


* add highs to requirements

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* docs: add changelog entry

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* fix: get results with infeasible termination status instead of RuntimeError

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* feat: add Binary constraint to prevent energy losses, and start new test against negative prices

Signed-off-by: F.N. Claessen <felix@seita.nl>

* fx: avoid double solving

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* style: fix HiGHS capitalization

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* remove HiGHS from requirements

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* remove dependency

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add dependency back

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* docs: document how to install HiGHS

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add HIghs to Dockerfile

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* remove extra lines

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* fix typos

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* load solution when termination_condition!=infeasible

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* refactor: run data preparation step in a different method

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* refactor: create run_device_scheduler function to return model and results objects and using it in the device_scheduler function

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add asserts and docstring in test_battery_solver_day_3

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add documentation for the constraints device_up_derivative_sign and device_down_derivative_sign

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* install HiGHS in the CI testing env

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* address some textual changes

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* fx CBC capitalization

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* fix grammar

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* check if there are results in a more robustly

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* little fixes

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* improve docstring of _prepare

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* fix definition of M

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* improve test

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* reorder installation instructions for CBC and HiGHS

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add production price fixture

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* fix loading results twice

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* remove TODO

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* increment StorageScheduler version

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add changelog entry

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* fix conftest

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* update test_hashing with a new hash due to the change in the version of the StorageScheduler

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* feat: in play mode, allow showing any sensor on the asset page (#740)

This PR uses the FlexMeasures play mode to allow showing sensors across organisations. When showing simulation results, we want to be able to show results from different scenarios (stored under different accounts) on the same asset page. For example, when comparing a scenario against some other benchmark scenario.


* feat: in play mode, allow showing any sensor on the asset page

Signed-off-by: F.N. Claessen <felix@seita.nl>

* fix checking for config value

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* more consistent naming of account variable

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* chore: rename variable of accessible sensors for readability

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* docs: mention this new possibility in function docstring

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* fix: do not fail if sensors are not accessible, log a warning for them as well

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* docs: changelog entry

Signed-off-by: F.N. Claessen <felix@seita.nl>

* docs: changelog warning

Signed-off-by: F.N. Claessen <felix@seita.nl>

---------

Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Co-authored-by: Nicolas Höning <nicolas@seita.nl>

* fix fixture

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* fix: changelog

Signed-off-by: F.N. Claessen <felix@seita.nl>

* revert (the refactoring part of aa32839): prepend model to the return tuple of the device_scheduler; this prevents duplicating a large docstring at the cost of introducing a breaking change in the function signature of the device_scheduler (but no external code uses that function to the best of my knowledge) (#781)

Signed-off-by: F.N. Claessen <felix@seita.nl>

* clean code and latitude, longitude and  account_id to get_or_create_generic_asset

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add highspy to test.txt and test.in

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* fix: typos

Signed-off-by: F.N. Claessen <felix@seita.nl>

* fix: test that needs to get the battery object

Signed-off-by: F.N. Claessen <felix@seita.nl>

* fix: typo

Signed-off-by: F.N. Claessen <felix@seita.nl>

* docs: clarify choice of SoC values in test

Signed-off-by: F.N. Claessen <felix@seita.nl>

* fix: test case 2 says it tests for no oscillation, which is only guaranteed when prefer-charging-sooner is true; otherwise, the solver will be completely indifferent to oscilations during the period with negative prices

Signed-off-by: F.N. Claessen <felix@seita.nl>

* style: streamline variable names in test

Signed-off-by: F.N. Claessen <felix@seita.nl>

* revert 48fd692: unnecessary after #695

Signed-off-by: F.N. Claessen <felix@seita.nl>

* style: black

Signed-off-by: F.N. Claessen <felix@seita.nl>

* remove installation of highspy as it's already being installed through the test requirements.

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* fix (docs): comment out cross reference to masked documentation page

Signed-off-by: F.N. Claessen <felix@seita.nl>

---------

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: Victor <victor@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Co-authored-by: Victor Garcia Reolid <victor@seita.nl>
Co-authored-by: Nicolas Höning <nicolas@seita.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants