Skip to content

Major Refactoring#249

Merged
chinmayshah99 merged 66 commits intodevfrom
test-capi
Aug 17, 2020
Merged

Major Refactoring#249
chinmayshah99 merged 66 commits intodevfrom
test-capi

Conversation

@chinmayshah99
Copy link
Copy Markdown
Member

@chinmayshah99 chinmayshah99 commented Aug 16, 2020 •
edited
Loading

Description

This is major refactoring of this library.
We have managed to

  • completely got rid of the C API architecture
  • added Python code thus allowing easy integration w/ other python code.
  • added doc strings
  • mypy support
  • moving of modules to a more intuitive manner into three sub-modules
  • moving of some tests to skip while we fix them

Fixes #150

Affected Dependencies

  • The DP submodule is upgraded to the latest SHA

How has this been tested?

  • make tests passes

Checklist

alejandrosame and others added 30 commits August 5, 2020 19:10
@alejandrosame
Disable distributions code
2b01e52
@alejandrosame
First proof of concept for internal aux class
6ef9cda
@alejandrosame
Update Count test
958d57f
@chinmayshah99
Merge pull request #222 from alejandrosame/refactoring
3449973 
[WIP] Refactoring
@alejandrosame
Refactor bounded functions
6885fb3
@chinmayshah99
Merge pull request #223 from alejandrosame/refactoring
84380ae 
[WIP] Refactoring: Bounded functions
@BenjaminDev
Added a more compact name type lookup. We might need to validate the …
e08c9ca 
…combinations as sum maybe not supported.
@chinmayshah99
Merge pull request #225 from BenjaminDev/count-refactor
a926b60 
[WIP] Added a more compact name and type lookup.
@chinmayshah99
added more epsilon property amd budget
acd5f45
@chinmayshah99
Merge pull request #228 from chinmayshah99/refactor-capi
3126da7 
added more epsilon property amd budget
@chinmayshah99
added error handing in intialization
05eeccf
@chinmayshah99
Merge pull request #229 from chinmayshah99/refactor-capi
62073bf 
Added error handing during initialization
@alejandrosame
Refactor builders error check
bcceec7
@alejandrosame
Bind more functions
e6069b0
@alejandrosame
Unifiy helper build function
b9c881c
@chinmayshah99
Merge pull request #230 from alejandrosame/refactoring
eea4927 
[WIP] Refactoring: Bind extra methods
@alejandrosame
Allow setting delta param
ad24725
@chinmayshah99
added error checking in partial result
2bc904f
@chinmayshah99
Merge pull request #231 from chinmayshah99/refactor-capi
cf181bb 
added error handling in partial result
@alejandrosame
Generalise algorithm declaration
d4aa0a7
@alejandrosame
Fix merge conflict
1dc6589
@alejandrosame
Fix delta arg position
cda7909
@alejandrosame
Fix Count result call
edfddd8
@alejandrosame
Define algorithms submodule
2d8c86a
@alejandrosame
Start defining new Python API
c729da6
@chinmayshah99
Merge pull request #232 from alejandrosame/refactoring
016d7c5 
[WIP] Refactoring: Generalise algorithm declaration
@alejandrosame
Rename pydp.so to _pydp.so
d0f8ce3
@alejandrosame
Update Python API
537032f
@chinmayshah99
Merge pull request #237 from alejandrosame/refactoring
655d351 
[WIP] Refactoring: Python interface
@chinmayshah99
removed delta as it can't be used for laplacian
fe7a202
@chinmayshah99 chinmayshah99 added the Type: Refactor 🔨 A complete overhaul of a file, feature, or codebase label Aug 16, 2020
alejandrosame
alejandrosame reviewed Aug 16, 2020
View reviewed changes
Comment thread pydp/algorithms/laplacian/_bounded_algorithms.py Outdated
alejandrosame
alejandrosame reviewed Aug 16, 2020
View reviewed changes
Comment thread tests/base/test_percentile.py Outdated
alejandrosame
alejandrosame reviewed Aug 16, 2020
View reviewed changes
Comment thread pydp/algorithms/_algorithm.py Outdated
alejandrosame
alejandrosame previously approved these changes Aug 16, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@alejandrosame alejandrosame left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I mostly noticed typos that didn't catch my eye while coding and checking previous PRs. I guess other reviewers could have more structural comments!

alejandrosame
alejandrosame reviewed Aug 17, 2020
View reviewed changes
Comment thread pydp/algorithms/laplacian/_bounded_algorithms.py Outdated
alejandrosame
alejandrosame previously approved these changes Aug 17, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@alejandrosame alejandrosame left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I catched a minor typo after the last changes, but otherwise it LGTM!

@chinmayshah99 chinmayshah99 merged commit 339fe35 into dev Aug 17, 2020
@chinmayshah99 chinmayshah99 deleted the test-capi branch August 17, 2020 21:15
@alejandrosame alejandrosame mentioned this pull request Aug 17, 2020
Closed
2 tasks
@chinmayshah99 chinmayshah99 mentioned this pull request Aug 17, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@replomancer replomancer Awaiting requested review from replomancer

@simcof simcof Awaiting requested review from simcof

@BenjaminDev BenjaminDev Awaiting requested review from BenjaminDev

@jeamick jeamick Awaiting requested review from jeamick

1 more reviewer

@alejandrosame alejandrosame alejandrosame left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Type: Refactor 🔨 A complete overhaul of a file, feature, or codebase

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Code duplication in C API

3 participants

@chinmayshah99 @alejandrosame @BenjaminDev