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

Api #13

Merged
merged 4 commits into from
Jul 28, 2021
Merged

Api #13

merged 4 commits into from
Jul 28, 2021

Conversation

vvrubel
Copy link
Owner

@vvrubel vvrubel commented Jul 25, 2021

внесены правки в настройки проекта, служебные файлы, README, а также модели ответа api и операции с базой данных. Опять большой poetry.lock

@vvrubel vvrubel requested a review from churnikov July 25, 2021 17:15
@aigulkhkmv aigulkhkmv self-requested a review July 27, 2021 10:56

class Compound(BaseModel):
CID: int
MolecularFormula: str
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ты не вводила тип smiles или что-то вроде того?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Ты не вводила тип smiles или что-то вроде того?

ну в pydantic модели этой оно есть как CanonicalSMILES, но отдельный тип не вводила, потому что я хз как ему его от лычной строки отличать

MolecularWeight: float
CanonicalSMILES: str
InChI: str
IUPACName: Optional[str] = None
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
Owner Author

Choose a reason for hiding this comment

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

Название по ИЮПАК может и не встретиться?

угу, это чисто если в данных скачанных с pubchem у конкретного вещества его не было

RotatableBondCount: int
AtomStereoCount: int
BondStereoCount: int
Volume3D: Optional[float] = None
Copy link
Collaborator

Choose a reason for hiding this comment

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

У предыдущих параметров всегда есть значение, а у Volume3D может не быть?

Copy link
Owner Author

Choose a reason for hiding this comment

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

У предыдущих параметров всегда есть значение, а у Volume3D может не быть?

анализ коллекции показал, что так - видимо как-то сложнее его считать и для некоторых соединений pubchem его не предоставляет 🤷‍♀️

StandardDeviation: Optional[float] = None


class CompoundSummary(BaseModel):
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
Owner Author

Choose a reason for hiding this comment

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

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

у меня тут 2 ручки - первая выдает просто все записи из коллекции, которые подошли по подструктуре (ограниченные параметрами пагинации), вторая берет данные подструктурного поиска и считает для них среднее и отклонение. Короче ты права)

return [skip_, limit_]
def run_search(smiles: str) -> List[str]:
"""
Функция генерирует объект молекулы для работы rdkit, после этот объект используется для
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
Owner Author

Choose a reason for hiding this comment

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

У тебя во всем репо докстринги на русском?

да, я просто в какой-то момент устала грамматику проверять и изменила на русский - в будущем планирую поменять, но после реализации всей логики

result = run_search(smiles)
pipeline = paging_pipeline(result, skip, limit)
cursor = properties.aggregate(pipeline)
return cursor


@timer
Copy link
Collaborator

Choose a reason for hiding this comment

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

Это чтобы в логи записывать?
Кстати, вроде логов у тебя нет, было бы хорошо их добавить, чтобы потом понимать, что происходило, если что-то сломается. С помощью loguru это просто сделать

Copy link
Owner Author

Choose a reason for hiding this comment

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

Это чтобы в логи записывать?
Кстати, вроде логов у тебя нет, было бы хорошо их добавить, чтобы потом понимать, что происходило, если что-то сломается. С помощью loguru это просто сделать

надо бы.. видимо в пул задач надо записать это

type=click.Choice(["PROD", "DEV"], case_sensitive=False),
help="Опция, позволяющая выбирать конфигурационный файл, содержащий переменные окружения, "
"который также определяет настройки базы данных.",
"--database", type=click.Choice(["PROD", "DEV"], case_sensitive=False),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Если выбрать PROD, и воспользоваться загрузкой данных в монгу, то человек будет сразу в прод писать?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Если выбрать PROD, и воспользоваться загрузкой данных в монгу, то человек будет сразу в прод писать?

ну тк база используется локальная, то у него просто в донги две базы: одна – тестовая, вторая – побогаче, типа прод. Вообще надо бы у Никиты после отпуска спросить, как это лучше настроить, но пока я ориентируюсь на то, что база локальная

)
@click.option(
"--drop",
is_flag=True,
help="Если опция '--drop' указана, то коллекция будет очищена.",
help='Если опция "--drop" указана, то база будет очищена.',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Коллекция или база? В хелпе написано "--drop" удаляет все коллекции в базе

Copy link
Owner Author

Choose a reason for hiding this comment

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

Коллекция или база? В хелпе написано "--drop" удаляет все коллекции в базе
есть 2 базы: тестовая и основаная - в них все три коллекции по сути одинаковые, только разное количество документов (у меня в тестовой 100тыс, во второй 2 млн). Тестовая нужна, чтобы не испортить чего случайно, иначе повторная загрузка большого количества документов и расчет фингерпринтов для такой базы будут очень долгими


def drop_db(db: Database) -> None:

def drop_db(setup: Settings) -> None:
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
Owner Author

Choose a reason for hiding this comment

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

А зачем это нужно? Я так понимаю, это делается для создания уникальных индексов? Можно кейс использования, пожалуйста

это как раз про предыдущий коммент, где записи загружаются в базу: иногда может хотеться удалить все (если что-то там подпортил) и загрузить в базу с теми же настройками набор данных. Эта функция драпает все и создает необходимые коллекции и индексы заново

@vvrubel vvrubel requested a review from aigulkhkmv July 27, 2021 12:15
@vvrubel vvrubel merged commit 90c4040 into master Jul 28, 2021
@vvrubel vvrubel deleted the api branch August 8, 2021 12:06
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.

None yet

2 participants