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

Decomoposing and code improvement #50

Merged
merged 10 commits into from
Apr 3, 2023
Merged

Decomoposing and code improvement #50

merged 10 commits into from
Apr 3, 2023

Conversation

Roman223
Copy link
Collaborator

@Roman223 Roman223 commented Mar 28, 2023

Networks and nodes
Classes were separated into files

Test for models implemented.

2. Tests and other files changed appropriately.
3. Small fix in pyproject.toml.
Networks: decomposing, code improvement;
Tests:
Update because of new soft structure;
New test for models without pickle path.
@Roman223 Roman223 changed the title Refactoring and code improvement (#34); New test for models. Decomosing and code improvement (#34); New test for models. Mar 29, 2023
@Roman223 Roman223 changed the title Decomosing and code improvement (#34); New test for models. Decomoposing and code improvement (#34); New test for models. Mar 29, 2023
@Roman223 Roman223 changed the title Decomoposing and code improvement (#34); New test for models. Decomoposing and code improvement (#34) Mar 29, 2023
@Roman223 Roman223 changed the title Decomoposing and code improvement (#34) Decomoposing and code improvement #34 Mar 29, 2023
@Roman223 Roman223 linked an issue Mar 29, 2023 that may be closed by this pull request
@Roman223 Roman223 changed the title Decomoposing and code improvement #34 Decomoposing and code improvement Mar 29, 2023
Copy link
Collaborator

@Anaxagor Anaxagor left a comment

Choose a reason for hiding this comment

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

Update tutorials files also

Copy link
Collaborator

@jrzkaminski jrzkaminski left a comment

Choose a reason for hiding this comment

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

Please also update docs, by changing the auto-generated API section.
You can find an instruction on updating documentation here:
https://bamt.readthedocs.io/en/latest/getting_started/contribution.html

@nicl-nno nicl-nno self-requested a review March 31, 2023 07:58
bamt/builders.py Outdated Show resolved Hide resolved
bamt/builders.py Outdated
Comment on lines 128 to 131
if type in ['disc_num', 'disc']:
Node = nodes.DiscreteNode(name=vertex)
node = discrete_node.DiscreteNode(name=vertex)
elif type == 'cont':
Node = nodes.GaussianNode(name=vertex, regressor=regressor)
node = gaussian_node.GaussianNode(name=vertex, regressor=regressor)

Choose a reason for hiding this comment

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

Возможно, вот эту часть стоит переработать в виде Factory.

bamt/builders.py Outdated Show resolved Hide resolved
@@ -170,7 +169,7 @@ def calculate_weights(self, discretized_data: pd.DataFrame):
"""
import bamt.utils.GraphUtils as gru
if not all([i in ['disc', 'disc_num']
for i in gru.nodes_types(discretized_data).values()]):
for i in gru.nodes_types(discretized_data).values()]):

Choose a reason for hiding this comment

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

Для большей читаемости лучше gru.nodes_types(discretized_data).values() вынести в переменную.

И переработать в all([node_type in discrete_types for node_type in nodes_types])

Можно в целом вынести такую проверку в функцию вроде is_discrete - явно где-то ещё может понадобится.

bamt/networks/base.py Outdated Show resolved Hide resolved
bamt/networks/base.py Outdated Show resolved Hide resolved
bamt/nodes/base.py Outdated Show resolved Hide resolved
1. Get rid of from "bamt.nodes import *"
2. Description for group 1 added.
3. Capitalized variables were renamed.
nodes:
1. Duplicated code removed.
networks:
1. Naming error fixed.
2. Small code fixes.
@Roman223 Roman223 merged commit c8dfc97 into master Apr 3, 2023
@Roman223 Roman223 deleted the refactoring branch April 3, 2023 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Decomoposing the modules
4 participants