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.

Sign up for GitHub

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

Jump to bottom

Refactor sync.py module #56

Merged
merged 6 commits into from
Aug 24, 2021
Merged

Refactor sync.py module #56

merged 6 commits into from
Aug 24, 2021

Conversation

jankrepl
Copy link
Contributor

@jankrepl jankrepl commented Aug 18, 2021 •
edited
Loading

Closes #52

What was done

  • Remove everything that is not directly used inside of download_dataset_parallel
  • Remove corresponding tests

See the coverage after this PR

---------- coverage: platform darwin, python 3.8.11-final-0 ----------
Name                     Stmts   Miss Branch BrPart  Cover   Missing
--------------------------------------------------------------------
src/atldld/__init__.py       2      0      0      0   100%
src/atldld/base.py         101      0     32      0   100%
src/atldld/cli.py           20     20      2      0     0%   17-61
src/atldld/sync.py          35      0     12      0   100%
src/atldld/utils.py        121     35     48      5    67%   57, 114, 126, 131-151, 170-184, 512, 532-552
src/atldld/version.py        1      0      0      0   100%
--------------------------------------------------------------------
TOTAL                      280     55     94      5    79%

@jankrepl
Copy link
Contributor Author

@Stannislav @EmilieDel Feel free to review

EmilieDel
EmilieDel approved these changes Aug 18, 2021
View reviewed changes
Copy link
Contributor

@EmilieDel EmilieDel left a comment

Choose a reason for hiding this comment

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

That is what we call cleaning :) LGTM! Thanks @jankrepl

docs/source/allen.rst Show resolved Hide resolved
Stannislav
Stannislav reviewed Aug 18, 2021
View reviewed changes
Copy link
Contributor

@Stannislav Stannislav left a comment

Choose a reason for hiding this comment

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

More deletions! Feels good 😆

Anyway, left two small comments.

README.md Outdated Show resolved Hide resolved
docs/source/allen.rst Outdated Show resolved Hide resolved
@Stannislav Stannislav mentioned this pull request Aug 24, 2021
Open
@Stannislav Stannislav merged commit 5b33120 into BlueBrain:main Aug 24, 2021
@Stannislav Stannislav mentioned this pull request Aug 24, 2021
Merged
@jankrepl jankrepl mentioned this pull request Aug 26, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Stannislav Stannislav Stannislav approved these changes

@EmilieDel EmilieDel EmilieDel approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Refactor sync.py module
3 participants
@jankrepl @Stannislav @EmilieDel