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

Version variable added #1118

Merged
merged 3 commits into from
Jul 13, 2023
Merged

Version variable added #1118

merged 3 commits into from
Jul 13, 2023

Conversation

nicl-nno
Copy link
Collaborator

@nicl-nno nicl-nno commented Jul 6, 2023

Version variable is added to support the following snippet:

from fedot import __version__
print(__version__)

Сaused by:

image

@aim-pep8-bot
Copy link

aim-pep8-bot commented Jul 6, 2023

Hello @nicl-nno! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 3:1: F403 'from fedot import ' used; unable to detect undefined names
Line 3:1: F401 'fedot.
' imported but unused
Line 4:1: F401 'fedot.version.version' imported but unused

Comment last updated at 2023-07-13 13:32:45 UTC

@codecov
Copy link

codecov bot commented Jul 6, 2023

Codecov Report

Merging #1118 (b55b766) into master (cda892d) will increase coverage by 0.24%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1118      +/-   ##
==========================================
+ Coverage   78.32%   78.56%   +0.24%     
==========================================
  Files         130      131       +1     
  Lines        9321     9323       +2     
==========================================
+ Hits         7301     7325      +24     
+ Misses       2020     1998      -22     
Impacted Files Coverage Δ
fedot/__init__.py 100.00% <100.00%> (ø)
fedot/version.py 100.00% <100.00%> (ø)

... and 6 files with indirect coverage changes

Copy link
Collaborator

@MorrisNein MorrisNein left a comment

Choose a reason for hiding this comment

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

Вот тут есть рекомендации, как сохранить версию фреймворка только в одном месте (она дублируется в setup.py)

Думаю, стоит воспользоваться

@MorrisNein
Copy link
Collaborator

MorrisNein commented Jul 7, 2023

Можно создать version.py, в котором ничего не импортировать. Там задать переменную __version__. После этого импортировать эту переменную и в init, и в setup (только в setup импортировать по-хитрому, как в варианте 3 из примера выше).

Насколько я понимаю, в setup нужно, чтобы никакие компоненты фреймворка не импортировались. Поэтому для хранения версии нужен отдельный модуль, либо вообще текстовый файл.

@nicl-nno nicl-nno requested a review from MorrisNein July 12, 2023 21:48
@nicl-nno
Copy link
Collaborator Author

Вынес как ты предложил. Надеюсь это в pypi сработает.

@nicl-nno nicl-nno requested a review from gkirgizov July 13, 2023 10:49
Copy link
Collaborator

@MorrisNein MorrisNein left a comment

Choose a reason for hiding this comment

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

Попробовал сделать локальный build, получилось.

Попробовал загрузить на test.pypi.org - не получилось с вот такой ошибкой:

ERROR    HTTPError: 400 Bad Request from https://test.pypi.org/legacy/
         Invalid value for requires_dist. Error: Can't have direct dependency: 'thegolem @ git+https://github.com/aimclub/GOLEM.git@stable#egg=thegolem'

С реальным PyPI не возникает такой проблемы?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Теперь сюда нужно добавить строку, чтобы удовлетворить цели PR

from fedot.version import __version__

Copy link
Collaborator

Choose a reason for hiding this comment

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

да, в PyPi нельзя иметь завиимости такого вида. Поэтому для релизов зависимость отт ГОЛЕМа ставится на конкреттную версию

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Теперь сюда нужно добавить строку, чтобы удовлетворить цели PR

Добавил.

@nicl-nno nicl-nno requested a review from MorrisNein July 13, 2023 13:33
@nicl-nno nicl-nno merged commit 1775411 into master Jul 13, 2023
5 checks passed
@nicl-nno nicl-nno deleted the add-version-variable branch July 13, 2023 13:55
kasyanovse pushed a commit that referenced this pull request Jul 31, 2023
* Version variable added to fedot module
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.

4 participants