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

ihategit #33

Open
wants to merge 55 commits into
base: dev
Choose a base branch
from
Open

ihategit #33

wants to merge 55 commits into from

Conversation

aleph23
Copy link
Owner

@aleph23 aleph23 commented May 3, 2024

No description provided.

Krakitten and others added 30 commits February 21, 2020 11:43
Much better results. Empty and Cluttered songs have been almost eliminated (used to be a majority with my data).
reduced overfitting
Sourcery refactored MEinkitten branch
…o_pytorch

Sweep: Convert new master branch to pytorch
Much better results. Empty and Cluttered songs have been almost eliminated (used to be a majority with my data).

Co-authored-by: TheLastLionTurtle <[email protected]>
* Added noise in input

Much better results. Empty and Cluttered songs have been almost eliminated (used to be a majority with my data).

* Added noise layer

reduced overfitting

---------

Co-authored-by: TheLastLionTurtle <[email protected]>
aleph23 added 24 commits April 17, 2024 01:42
Missed chanegs from last PR
add back noise_rate
restore prior Pytorch migration
	modified:   .gitignore
	new file:   tests.py
	modified:   train.py
…to picklekitten

* 'picklekitten' of https://github.com/aleph23/Composer:
  Delete .vscode directory
  Delete .vs directory
  Delete .idea directory
  Update sweep-template.yml (#32)
@aleph23 aleph23 marked this pull request as ready for review May 3, 2024 09:02
Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @aleph23 - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟡 General issues: 9 issues found
  • 🟢 Security: all looks good
  • 🟡 Testing: 3 issues found
  • 🟢 Complexity: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

@@ -6,24 +6,31 @@
"""

import argparse
import itertools
Copy link

Choose a reason for hiding this comment

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

suggestion (code_refinement): Consider removing unused import if 'itertools' is not utilized in the module.

Suggested change
import itertools
import itertools

@@ -251,7 +263,7 @@ def update_mouse_click(mouse_pos):
mouse_pressed = 2


def apply_controls():
def apply_controls() -> object:
Copy link

Choose a reason for hiding this comment

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

suggestion (code_clarification): Explicit return type hint 'object' is too generic; specify a more precise type if possible.

Suggested change
def apply_controls() -> object:
def apply_controls() -> None:

@@ -0,0 +1,476 @@
{
"cells": [
Copy link

Choose a reason for hiding this comment

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

suggestion (code_refinement): Consider using a more descriptive name for the notebook file.

The name 'GradiantVis.ipynb' could be a typo or may not fully describe the purpose of the notebook. Consider renaming it to something more descriptive, such as 'GradientVisualization.ipynb', if it is indeed about visualizing gradients.

"from keras.models import Model, load_model\n",
"from keras import backend as K\n",
"import numpy as np\n",
"import params\n",
Copy link

Choose a reason for hiding this comment

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

issue (code_refinement): Avoid duplicate imports.

The module 'params' is imported twice. Removing duplicate imports can clean up the code and improve its readability.

Comment on lines +62 to +66
"# User constants\n",
"dir_name = 'results/history/'\n",
"sub_dir_name = 'basic'\n",
"num_measures = 16\n",
"use_pca = True"
Copy link

Choose a reason for hiding this comment

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

suggestion (code_refinement): Consider using a configuration file or environment variables for constants.

Hardcoding constants in the script can lead to difficulties in changes or different environments. Using a configuration file or environment variables can make the code more flexible and easier to manage across different environments or conditions.

Suggested change
"# User constants\n",
"dir_name = 'results/history/'\n",
"sub_dir_name = 'basic'\n",
"num_measures = 16\n",
"use_pca = True"
import json
# Load configuration
with open('config.json', 'r') as config_file:
config = json.load(config_file)
dir_name = config['dir_name']
sub_dir_name = config['sub_dir_name']
num_measures = config['num_measures']
use_pca = config['use_pca']

Comment on lines +34 to +36
if samples: # If there are samples, check their structure
assert isinstance(samples[0], np.ndarray), f"Test ID {test_id}: Each sample should be a numpy array."
assert samples[0].shape == (samples_per_measure, num_notes), f"Test ID {test_id}: Incorrect shape of sample."
Copy link

Choose a reason for hiding this comment

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

issue (code-quality): Avoid conditionals in tests. (no-conditionals-in-tests)

ExplanationAvoid complex code, like conditionals, in test functions.

Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:

  • loops
  • conditionals

Some ways to fix this:

  • Use parametrized tests to get rid of the loop.
  • Move the complex logic into helpers.
  • Move the complex part into pytest fixtures.

Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.

Software Engineering at Google / Don't Put Logic in Tests

@@ -101,6 +99,7 @@ def plot_losses(scores, f_name, on_top=True):


def save_training_config(num_songs, model, learning_rate):
# sourcery skip: use-fstring-for-concatenation
Copy link

Choose a reason for hiding this comment

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

issue (code-quality): Unused skip comment (unused-skip-comment)

@@ -117,7 +124,7 @@
audio_pause = False


def audio_callback(in_data, frame_count, time_info, status):
def audio_callback(in_data,frame_count,time_info,status):
Copy link

Choose a reason for hiding this comment

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

issue (code-quality): We've found these issues:


Explanation

The quality score for this function is below the quality threshold of 25%.
This score is a combination of the method length, cognitive complexity and working memory.

How can you solve this?

It might be worth refactoring this function to make it shorter and more readable.

  • Reduce the function length by extracting pieces of functionality out into
    their own functions. This is the most important thing you can do - ideally a
    function should be less than 10 lines.
  • Reduce nesting, perhaps by introducing guard clauses to return early.
  • Ensure that variables are tightly scoped, so that code using related concepts
    sits together within the function rather than being scattered.


self.clock = pygame.time.Clock()

def update(self, events):
Copy link

Choose a reason for hiding this comment

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

issue (code-quality): Low code quality found in TextInput.update - 15% (low-code-quality)


ExplanationThe quality score for this function is below the quality threshold of 25%.
This score is a combination of the method length, cognitive complexity and working memory.

How can you solve this?

It might be worth refactoring this function to make it shorter and more readable.

  • Reduce the function length by extracting pieces of functionality out into
    their own functions. This is the most important thing you can do - ideally a
    function should be less than 10 lines.
  • Reduce nesting, perhaps by introducing guard clauses to return early.
  • Ensure that variables are tightly scoped, so that code using related concepts
    sits together within the function rather than being scattered.

train.py Show resolved Hide resolved
	deleted:    .vs/VSWorkspaceState.json
	deleted:    .vscode/settings.json
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.

3 participants