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

Make Serializer extendable for dependent projects #969

Merged
merged 20 commits into from
Nov 9, 2022

Conversation

gkirgizov
Copy link
Collaborator

Existing Serializer was closed for extension with new classes and so couldn't be reused in GOLEM.

This PR resolves this by allowing registering serializable classes, so that they could be serialized together with default classes.

An example is a serialization of custom graphs that are stored in the OptHistory.

Main changes:

  • Add register_serializable decorator for custom serializable classes
  • Move Operation class (which is Fedot-specific) to use this decorator

@gkirgizov gkirgizov added the infrastructure Related to non-core aspects of the framework: multiprocessing, serialisation, logging, etc. label Nov 7, 2022
@gkirgizov gkirgizov self-assigned this Nov 7, 2022
@MorrisNein MorrisNein self-requested a review November 7, 2022 09:27
fedot/core/serializers/serializer.py Outdated Show resolved Hide resolved
fedot/core/serializers/serializer.py Outdated Show resolved Hide resolved
test/unit/serialization/test_custom_serialize.py Outdated Show resolved Hide resolved
decoded_self = obj.from_json(dumped_self)
decoded_srz = json.loads(dumped_srz, cls=Serializer)

assert decoded_self == decoded_srz == deepcopy(obj)
Copy link
Collaborator

Choose a reason for hiding this comment

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

А если obj.from_json(dumped_srz)?
Или json.loads(dumped_self, cls=Serializer)?

Они тоже будут равны? Аналогично про метод снизу

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Думал про это, но у них немного разные сигнатуры.
json.loads возвращает str,
а вот obj.to_json & obj.from_json работают с Dict[str, Any].

Так что нет, равны не будут. Чтобы этого добиться, придется Serializer менять во многих местах, так как он ожидает именно словарики от методов сериализации.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Добавлю пару строк в тесте, чтобы это подчеркнуть

Copy link
Collaborator

Choose a reason for hiding this comment

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

А в теории же obj.to_json & obj.from_json могут работать с чем угодно и выдавать тоже что угодно? Или есть какой-то чёткий контракт? Соре, если не заметил в коде

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Есть контракт, сигнатура определена в EncodeCallable & DecodeCallable

Copy link
Collaborator

Choose a reason for hiding this comment

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

Пробовал прогонять эти куски кода через статические анализаторы? А вообще длинная цепочка типов получается

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Нет, не прогонял, довольствуюсь встроенными PyCharm проверками. Какой-нибудь mypy у меня не настроен

fedot/core/serializers/serializer.py Outdated Show resolved Hide resolved
@gkirgizov gkirgizov mentioned this pull request Nov 9, 2022
10 tasks
fedot/core/serializers/serializer.py Show resolved Hide resolved
fedot/core/serializers/serializer.py Show resolved Hide resolved
fedot/core/operations/operation.py Outdated Show resolved Hide resolved
@gkirgizov gkirgizov force-pushed the serialization-make-extendable branch from 07da88e to 2060cdb Compare November 9, 2022 11:39
@gkirgizov gkirgizov force-pushed the serialization-make-extendable branch from 0d45e2f to db4dd8f Compare November 9, 2022 14:01
@gkirgizov gkirgizov merged commit b2bc946 into master Nov 9, 2022
@gkirgizov gkirgizov deleted the serialization-make-extendable branch November 9, 2022 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Related to non-core aspects of the framework: multiprocessing, serialisation, logging, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants