Skip to content
This repository has been archived by the owner on Jun 3, 2020. It is now read-only.

Code cleaning #64

Merged
merged 13 commits into from
May 3, 2018
Merged

Code cleaning #64

merged 13 commits into from
May 3, 2018

Conversation

delhomer
Copy link
Contributor

@delhomer delhomer commented Apr 27, 2018

Some cleaning operations to prepare the 0.4 release:

  • version updatting in setup.py
  • some useless modules deletion, some module renaming
  • code reading (in processing)
  • update README.md

Fixes the issue #63 .

@delhomer delhomer mentioned this pull request Apr 27, 2018
5 tasks
Update docstrings and comments; remove print statements; remove
useless legacy functions in `utils.py`.
The dropout rate was wrongly implemented, as putting it as a command
argument did not change anything: the corresponding value was not
taken into account, and a 75% dropout rate was considered. This commit
fixes the issue, by setting dropout as an attribute of class
ConvolutionalNeuralNetwork and its derivatives.
@delhomer delhomer changed the title [WIP] Code cleaning Code cleaning May 2, 2018
@delhomer delhomer requested a review from garaud May 2, 2018 17:11
Copy link
Contributor

@garaud garaud left a comment

Choose a reason for hiding this comment

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

minor comments

setup.py Outdated
@@ -6,7 +6,7 @@
version = line.strip().split('=')[1].strip(' \'"')
break
else:
version = '0.3.2'
version = '0.4'
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe write also the version number in the __init__.py file. What to you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes you're write, it seems to be a good practice. Should I modify the setup.py too, or should we keep the version in both scripts?

Copy link
Contributor

Choose a reason for hiding this comment

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

we can keep the version in both scripts. I'll look for another open source projects how they deal with that.

+ numpy
+ pandas
+ PIL
+ tensorflow
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe mention the setup.py file with the dependencies and the requitements dev file if needed.

@delhomer
Copy link
Contributor Author

delhomer commented May 3, 2018

Some new commits have been added to fit your remarks. If everything is OK, we can merge, and release the new code version.

@delhomer delhomer merged commit ae6a121 into master May 3, 2018
@delhomer delhomer deleted the code_cleaning branch May 3, 2018 15:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants