Temporary queue to prevent duplication I am making a Reddit bot and to prevent my bot from posting to the same comment twice, instead of making a database, I decided I would rather create a temporary list (or "queue") that once appended to, checks to ensure the maximum length of the queue hasn't been exceeded. Once exceeded, however, the queue is cleared.

class TemporaryQueue:
    def __init__(self, max_items=10):
        self._queue = []
        self.max_items = max_items

    def __repr__(self):
        return "<TemporaryQueue max_items={}>".format(self.max_items)

    def __iter__(self):
        yield from self._queue

    def __eq__(self, other):
        return other in self._queue

    def _exceed_check(self):
        if len(self._queue) > self.max_items:
            return True
        return False

    def append(self, value):
        if self._exceed_check():
            self._queue.clear()

        return self._queue.append(value)

Example usage:

for submission in reddit.subreddit("funny").stream.submissions():
    queue = TemporaryQueue()

    if submission.id not in queue:
        if "foo" in title:
            submission.reply("Bar!")
            queue.append(submission.id)

collections.deque

Why not use a collections.deque with a max_len?

from collections import deque

for submission in reddit.subreddit("funny").stream.submissions():
    queue = deque(max_len=10)

    if submission.id not in queue:
        if "foo" in title:
            submission.reply("Bar!")
            queue.append(submission.id)


Command-query segregation principle for an authenticator The Authenticate method violates the command-query segregation principle, right? How can I adhere to the command-query segregation principle here?

public class Authenticator : IAuthenticator
{
    public bool Authenticated
    {
        get { return HttpContext.Current.User.Identity.IsAuthenticated; }
    }

    public bool Authenticate(string username, string password)
    {
        var authenticated = FormsAuthentication.Authenticate(username, password);
        if (authenticated)
            FormsAuthentication.SetAuthCookie(username, false);
        return authenticated;
    }

    public void Logout()
    {
       FormsAuthentication.SignOut();
    }
}

Since it's been several days and this question has not been answered, I will give it a go. I'm saying this not because making Authenticate follow Command-Query Separation Principle is hard, but rather I think this design may indicate a more serious problem with security configuration, and I'm not up to date with ASP.NET.

Command Query Separation

  The Authenticate method violates the command-query segregation principle no?

Since you seem to be familiar with Fowler's terminology, let's look at his definition of Command/Query Separation.

  The fundamental idea is that we should divide an object's methods into
  two sharply separated categories:

  Queries: Return a result and do not change the observable state of the system (are free of side effects).
  Commands: Change the state of a system but do not return a value.

Since in this case Authenticate definitely changes state, specifically the session state by setting a cookie, it is a command and should not return a value. You can achieve this, as suggested by @svick:

  you [can] just make Authenticate() return void and then use Authenticated instead.

That is:

public void Authenticate(string username, string password)
{
    // unused `authenticated` can be inlined
    if (FormsAuthentication.Authenticate(username, password))
        FormsAuthentication.SetAuthCookie(username, false);
}

And why not following CQS is a problem? Because Authenticate() returns a value, it is not immediately clear to the reader of the code if the writer of the code just checks the validity of the username/password pair or does what it does now, thus makes understanding the code harder.
Similarly someone looking at the IAuthenticator, which should look like this as you stated that it's a Header Interface:

public interface IAuthenticator
{
    bool Authenticated {get;}
    bool Authenticate(string username, string password);
    void Logout();
}

faces similar dilemma: Do I use Authenticate to just check for the validity of username/password, or does it do something more?

There is also another problem with returning bool from Authenticate(...), because Authenticate is an imperative word, and an Autheticated getter (read-only property) is also present, it could be that bool return value is an error code for the Authenticate command, that is you MUST check it immediately after calling the method and do something with it other than what you would to with the return value of Authenticated. For example Authenticated == false implies wrong password whereas Authenticate(username, password) == false could mean "Membership database is down". Of course error codes are a no-no.

Now I should also state that apparently MS APIs do not always follow CQS principle, as seen here.

But Command Query Separation should also make us reevaluate if the method underneath follows Single Responsibility Principle. A method like this would be used like this:

if (Authenticator.Authenticate(credentials)) {
   // continue to secured content
} else {
   // return to login page
}

We see that the if in Authenticate is duplicate, and increases the complexity of the code unnecessarily:

if (Authenticator.Validate(credentials)) {
   Authenticator.Login();
   // continue to secured content
} else {
   // return to login page
}

This division requires some further points to be made:

Currently Authenticator has two separate responsibilities: Checking if username/password pairs are valid, and keep user logged in across multiple requests.
FormsAuthentication.SetAuthCookie requires username, but you do not use it after checking whether username/password is valid, i.e. Authenticator.Login does not need it.
However your current implementation would need the username to be passed to Authenticator.Login, therefore you would need to modify the interface because of an implementation detail. That is IAuthenticator would not be a proper abstraction over FormsAuthentication.
This is rightly so, because even though Authenticator pretends it's not a static class, it does not encapsulate it's own state. You can observe this, as it does not have any state of its own and all the methods it calls from FormsAuthentication are static methods:

// Note that the no-argument constructor hides its dependency
// to `Thread`, `HttpContext` or something.
IAuthenticator authenticatorOne = new Authenticator();
IAuthenticator authenticatorTwo = new Authenticator();
authenticatorOne.Authenticate("Alice", "4L1(3'5 p455\/\/0rD");
authenticatorTwo.Authenticate("Bob", "808'5 p455\/\/0rD");
authenticatorTwo.Logout();

As seen here, having multiple instances of Authenticator (in a request? thread?) does not make any sense.

Naming Side Note

It is hard to parse Authenticator.Authenticated mentally, as the object of the Authenticate is absent. For example in IIdentity.IsAuthenticated it is clear that the identity is the authenticated thing. If we change members of IAuthenticator as follows:

public interface IAuthenticator
{
    bool Validate(string username, string password);

    bool SomeUserIsLoggedin {get;}
    void Login();
    void Logout();
}

Now it is clear that Login/Logout are opposite commands. SomeUserIsLoggedin checks whether Login has been called and Logout has not been called yet. And Login/Logut having no parameter matches with Some User being the object of Is Logged In, which altogether indicate that you do not care who the user is and care just that he logged in.

This way Validate is more clearly seen to be separable from the rest of the IAuthenticator members.

Header Interfaces Side Note

If you mean Header Interface as in the Fowler's dichotomy of Header/Role interfaces, it is rather an anti pattern, as they violate the ISP:

  Robert Martin talks about this topic as the Interface Segregation
  Principle: role interfaces follow that principle but header interfaces
  do not.


Determine whether an integer is a palindrome The task

  Determine whether an integer is a palindrome. An integer is a
  palindrome when it reads the same backward as forward.

  Example 1:

  Input: 121 Output: true

  Example 2:

  Input: -121 Output: false Explanation: From left to right, it reads
  -121. From right to left, it becomes 121-. Therefore it is not a palindrome.

  Example 3:

  Input: 10 Output: false Explanation: Reads 01 from right to left.
  Therefore it is not a palindrome. Follow up:

  Could you solve it without converting the integer to a string?

My solution
with converting number to string

const isPalindrome = n => n >= 0 && Number([...`${n}`].reverse().join("")) === n;

console.log(isPalindrome(121));

My solution
without converting number to string

const isPalindrome2 = n => {
  if (n < 0) { return false; }
  let num = Math.abs(n);
  const arr = [];
  let i = 1;
  while (num > 0) {
    const min = num % (10 ** i);
    num = num - min;
    i++;
    arr.push(min);
  }
  i = i - 2;
  let j = 0;
  return n === arr.reduce((res, x) => {
    const add = (x/ (10 ** j)) * (10 ** i);
    res += add;
    i--;
    j++;
    return res;
  }, 0);
};

console.log(isPalindrome2(121));

Your second solution is awfully complicated for something simple.

You start out alright.  No negative number can be a palindrome, so we can discount those. However, 0 is always a palindrome, so you're discounting that while you shouldn't.

You do a lot of complicated math and use an array. You could use either, but shouldn't use both. There are a couple of mathematical approaches to solve this, but you should be able to use those without iterating over the individual numbers or splitting it up at all. The most obvious solution however, is using an array and direct comparisons, without any math.

Say we got 1221. Split it up. [1, 2, 2, 1]. Iterate over the array, comparing every nth character to the last-nth character. 0th to 3rd. 1st to 2nd.

Say we got 92429. Split it up. [9, 2, 4, 2, 9]. Ignore the middle character. Handle the rest like it's an even-length number.

Based on those 2 cases, you should be able to figure out a much simpler algorithm.

Note: This answers the explicit question. Implicitly, you should wonder whether arrays should be allowed for this challenge. After all, iterating over a string or an array, it's not that different. I strongly suspect they want you to use the math-only approach.

Another approach, which is somewhat math-based and you should beware of overflows, is simply reversing the number.
In pseudo-code, that would look something like this:

reverse = 0
while (number != 0) {
  reverse = reverse * 10 + number % 10;
  number /= 10;
}

Check the input versus its reversed number. If they are the same, it's a palindrome.

But this still uses extra memory to hold the additional integer we just created.

Can it be done without? Absolutely. But I'll leave that as an exercise for you.


Showing one category and hiding two others when one of three buttons is clicked How can I refactor this code? I know it's repetitive but I'm not sure how to fix it.

$(function() {
    $('#category-1-button a').bind('click', function() {
        $(this).css({opacity:'1'});
        $('#category-2-button a,#category-3-button a').css({opacity:'0.4'});
        $('#blog-headers').css({backgroundPosition: '0 0'});
        $('#category_2,#category_3').hide(0, function() {
            $('#category_1').show(0);
        });
    });
});

$(function() {
    $('#category-2-button a').bind('click', function() {
        $(this).css({opacity:'1'});
        $('#category-1-button a,#category-3-button a').css({opacity:'0.4'});
        $('#blog-headers').css({backgroundPosition: '0 -144px'});
        $('#category_1,#category_3').hide(0, function() {
            $('#category_2').show(0);
        });
    });
});

$(function() {
    $('#category-3-button a').bind('click', function() {
        $(this).css({opacity:'1'});
        $('#category-1-button a,#category-2-button a').css({opacity:'0.4'});
        $('#blog-headers').css({backgroundPosition: '0 -288px'});
        $('#category_1,#category_2').hide(0, function() {
            $('#category_3').show(0);
        });
    });
});

Give each #category-n-button a class like category_button.

Bind the handler in the each()[docs] method  so that you can use the index argument to calculate the background position.

Use this to reference the element that received the event in the click handler.

Use the not()[docs] method  to exclude the this element from the other category buttons when setting the opacity.

Use the filter()[docs] method  to show the category element that pertains to the index of .each() + 1.

$(function() {
    var categories = $('[id^="category_"]');
    var category_buttons_a = $('.category_button a').each(function( idx ) {
        $(this).bind('click', function() {
            $(this).css({opacity:'1'});
            category_buttons_a.not(this).css({opacity:'0.4'});
            $('#blog-headers').css({backgroundPosition: '0 ' + (idx * -144) + 'px'});
            categories.hide().filter( '#category_' + (idx + 1) ).show();
        });
    });
});


Making CSS rules for links in header more DRY I want to implement the cleanest amount of CSS through proper use of inheritance. All links need to be a shade of white, so lets say #fff

HTML

    BruxZir

    &equiv;

CSS

.header {
  background-color: #333;
  color: #fff;
  height: 36px;
  width: 100%;
  line-height: 36px;
}

/* should be replaced with image */
.headerLogo {
  display:inline;
  font-size: 24px;
  line-height: 36px;
  padding-left: 6px;
}
.headerLogo a{
  color:#fff;
  text-decoration:none;
}

.headerMenu{
  float:right;
  font-size: 36px;
  margin-right: 6px;
}

.headerMenu a {
  /* needs the same rules for color, font-size, no text-decoration */
}

Here is my Codepen

I am thinking that since many of these rules will apply to the rest of the links in the header, I can set this up somehow for better inheritance. Also wondering if I would be better served using s instead of
s for the logo and the menu?

The worst aspect of your code is the markup, not the CSS.  Unless you need the div as an actual styling hook, you can safely discard it.  After all, you're not styling it in any meaningful way (what you do have can just as easily be applied to the descendant tags).

    BruxZir
    &equiv;

However, the inclusion of semantic tags might be a better way to go:

BruxZir

        &equiv;


VBA Copy Cells Stable I'm very new to VBA and have put together a macro that currently does exactly what I want: copy cells from one worksheet to another, clear the cells copied, refresh my pivot tables and save the file.

Here is what I am using for it:

Sub Copyinfo()

    On Error GoTo Err_Execute

    Sheet1.Range("A17:R17").Copy
    Sheet2.Range("A2").Rows("2:2").Insert shift:=xlDown
    Sheet2.Range("A2").Rows("2:2").PasteSpecial Paste:=xlPasteValuesAndNumberFormats

    Sheet1.Range("C5:I16, Q5:R5").ClearContents
Err_Execute:
    If Err.Number = 0 Then MsgBox "Copied Data", vbInformation Else _
    MsgBox Err.Description

    ThisWorkbook.RefreshAll

    ThisWorkbook.Save
End Sub

What I want to know is whether this macro will be stable as I continue to add lines of data to it in the future. Essentially I want to minimize the risk of the file crashing or my data being erased. Otherwise, would anyone have any suggestions to offer me to improve this? Thank you.

few things that come to mind on a quick look:

You might consider naming this something other than "Copyinfo".  If it is archiving transactions, for example, you might name it "ArchiveTransactions".  If you do decide to stick with the name, you should go with the Pascal Case version: "CopyInfo".  While you're at it: it's always good to specify public instead of leaving it implicit.

You don't need to do Sheet2.Range("A2").Rows("2:2"), you can also do Sheet2.Range("2:2") or just Sheet2.Rows("2:2"), or (and most simply) Sheet2.Rows(2).  I like putting things in With blocks (although many people won't for just two things) because I like reducing code duplication.

As for the syntax with the If statement... If there is an Else, it should be on multiple lines, in my opinion.  The way you had it above with the line continuation seems to go out of it's way to stay on one line (which, it doesn't anyway!).  Clarity always wins over conciseness on my book.

If you use Application.ScreenUpdating as shown below, it will eliminate the screen flicker that can happen when you switch sheets, as well as speed up the performance (which, since this Sub doesn't do a whole lot, performance tuning isn't necessary).

Public Sub Copyinfo()

    Application.ScreenUpdating = False

    On Error GoTo Err_Execute

    Sheet1.Range("A17:R17").Copy

    With Sheet2.Rows(2)
        .Insert shift:=xlDown
        .PasteSpecial Paste:=xlPasteValuesAndNumberFormats
    End With

    Sheet1.Range("C5:I16, Q5:R5").ClearContents

Err_Execute:
    If Err.Number = 0 Then
        MsgBox "Copied Data", vbInformation
    Else
        MsgBox Err.Description
    End If

    ThisWorkbook.RefreshAll
    ThisWorkbook.Save

    Application.ScreenUpdating = True

End Sub


Checking if an xy position is within a rectangular bounds This is pretty straightforward, but is there a better way to do this?

function isWithinBounds(obj, pos) {
    var oXmin, oXmax, oYmin, oYmax;
    if (obj.startPos.x < obj.endPos.x) {
        oXmin = obj.startPos.x;
        oXmax = obj.endPos.x;
    }
    else {
        oXmin = obj.endPos.x;
        oXmax = obj.startPos.x
    }

    if (obj.startPos.y < obj.endPos.y) {
        oYmin = obj.startPos.y;
        oYmax = obj.endPos.y;
    }
    else {
        oYmin = obj.endPos.y;
        oYmax = obj.startPos.y;
    }

    return (pos.x > oXmin && pos.x < oXmax && pos.y > oYmin && pos.y < oYmax);

}

There is. You can use Math.min and Math.max to determine which is the smaller end and the larger end. This way, you avoid the if statements.

function isWithinBounds(obj, pos) {
    var oXmin = Math.min(obj.startPos.x, obj.endPos.x);
    var oXmax = Math.max(obj.startPos.x, obj.endPos.x);
    var oYmin = Math.min(obj.startPos.y, obj.endPos.y);
    var oYmax = Math.max(obj.startPos.y, obj.endPos.y);
    return ( oXmin < pos.x && pos.x < oXmax && oYmin < pos.y && pos.y < oYmax);
}


Bash script to send certain data from AWC EC2 to CloudWatch Here's a little bash script I wrote, which basically checks something on an Amazon EC2 instance and reports data to CloudWatch every 60 seconds. I'll run this inside a container. Everything works just fine, but I think it's clunky, doesn't write anything to stdout/stderr so no logs, and doesn't handle any errors. I am open to refactoring this or even writing it in Python, to make my docker image more efficient. Any suggestions, criticism is most welcome.

Here's the full script less some sensitive data that I've xxx'd:

#!/bin/bash

INSTANCEID=$(curl --silent http://169.254.169.254/latest/meta-data/instance-id/)
AZ=$(curl --silent http://169.254.169.254/latest/meta-data/placement/availability-zone)
REGION= $(echo $AZ | sed -e 's:\([0-9][0-9]*\)[a-z]*\$:\\1:')

URL="xxx"

putdata() {
    aws cloudwatch put-metric-data "some other stuff" $1 "xxx"
    sleep 60
}
while true; do
    HTTP_RESPONSE=$(curl --write-out "%{http_code}" --silent --output /dev/null "$URL")
    if [ "$HTTP_RESPONSE" = "200" ]; then
        putdata 0

    else
        putdata 1
    fi
done

EDIT: just realized that the sleep command runs as a separate PID inside the container, which is why the container takes quite a while to stop. Running with --init flag improves the exit workflow as it better reaps the processes upon exit but that's another unwanted PID.

Good

good variable names
nice indentation
good use of $()
double quoting $ variable interpolation

Challenges

I'm guessing some part of what you removed and xxxd has caused the putdata function to not do anything with the argument you are passing in.  If I guessed wrong, then you should remove the arguments so it doesn't cause someone else confusion.

Do you want this to be totally silent?    I'm not sure if the aws command produces any output here.  Adding a date command in the while loop would help debugging if you come back to the terminal later to figure out what happened.  I'd run this in a terminal under tmux for a few weeks before I started trying it in containers.  This sort of thing can have fun edge cases when some API falls over.  It can take a while to catch all of those.

It sounds like you'd like some logging.  That can be done in bash with redirection such as >> logfile.  Of course, there are more advanced choices.

I'd put a blank link before the while loop and get rid of the blank line inside of it.

You mention rewriting in python.  I wouldn't worry about it for anything this small.  But generally for AWS programming python is the winning choice since AWS provides their API that way.  Functionality may be available through the API before it is available in console.  Everything in the aws CLI command is built with this.  You can use jq to process JSON that comes back from running the aws command to do the same stuff in bash, but it is more work than python where you get a dictionary back and not have to parse any JSON.

Finally, using shellcheck is a good habit to get into when shell coding.


Delete companies with the least number of flights I have solved the task of

Delete the companies that made the least number of flights.

the visual table diagram is here. The exercise is here
This is my source code -it's "arrow antipattern" , what is more elegant solution?
the inner most counts num of flights and groups by company, this is used to compare and find the actual minimum, then just select company names and delete.
DELETE FROM company
WHERE  name IN (SELECT name
                FROM   (SELECT name
                        FROM   trip
                               INNER JOIN company
                                       ON trip.company = company.id
                        GROUP  BY name
                        HAVING Count(trip.id) = (SELECT Min(kolv) AS mini
                                                 FROM   (SELECT
                                                Count(trip.id) AS kolv
                                                         FROM   trip
                                                INNER JOIN company
                                                        ON trip.company
                                                           =
                                                           company.id
                                                         GROUP  BY name
                                                         ORDER  BY kolv)k)) o)

If you can with MySQL 8.0, use the WITH Common Table Expression to simplify this query.
https://dev.mysql.com/doc/refman/8.0/en/with.html
The idea is to extract out the nested queries into ones in the WITH statement before the main query and then use them just like tables in the main query.


Manipulating list values in python I wrote a function that I use to preprocess pandas dataframes before running them through a machine learning model. The function works perfectly, however I don't think it's the most pythonic way to write it.

This function accepts a list of words:

['here', 'is', 'a','sample', 'of','what','the','function','accepts']

def clean_text(x):

    stopwords_english = stopwords.words('english')

    for i,word in enumerate(x):
        if word.lower() in stopwords_english:
            x[i] = ''
        else:
            for punct in "/-'":
                x[i] = word.replace(punct, ' ')
            for punct in '&':
                x[i] = word.replace(punct, f' {punct} ')
            for punct in '?!.,"#$%\'()*+-/:;<=>@[\\]^_`{|}~' + '“”’':
                x[i] = word.replace(punct, '')
    return x

Here I am using enumerate to change the value inside of the list. I would have assumed a more pythonic way of doing it would be writing it as follows:

def clean_text(x):

    stopwords_english = stopwords.words('english')

    for word in x:
        if word.lower() in stopwords_english:
            word = ''
        else:
            for punct in "/-'":
                word = word.replace(punct, ' ')
            for punct in '&':
                word = word.replace(punct, f' {punct} ')
            for punct in '?!.,"#$%\'()*+-/:;<=>@[\\]^_`{|}~' + '“”’':
                word = word.replace(punct, '')
    return x

The function is being called as follows:

train['question_text'].progress_apply(lambda x: clean_text(x))

Where train is a pandas dataframe and 'question_text' is a column in the dataframe.

Is my current implementation the most pythonic way, or is there a better way?

This may be a good case for generator functions.  And splitting it into two parts might make things more flexible: first, remove the stopwords; and second, handle the punctuation.

Also, str.maketrans and str.translate can do the punctuation mapping.

def remove_stopwords(text_iterable, stopwords):
    for word in text_iterable:
        if word.lower() not in stopwords:
            yield word

def handle_punctuation(text_iterable, table):
    for word in text_iterable:
        yield word.translate(table)

# format is ("chars", "replacement")
mappings = (("/-'", ' '),
            ('&', ' & '),
            ('?!.,"#$%\'()*+-/:;<=>@[\\]^_`{|}~' + '“”’', None))

table = str.maketrans({c:v for k,v in mappings for c in k})

stopword_free = remove_stopwords(text, stopwords)
cleaned_text = handle_punctuation(stopword_free, table)

cleaned_text is a generator, use list(handle_punctuation(stopword_free, table)) if you need an actual list of words.


Filter dataframe based on unique values in multiple columns and drop rows based on index I'm wondering if there is a more efficient way of filtering a dataframe down based on certain unique values in several columns. Once I have it filtered down, I then want to extract keep one the largest value and I do this by dropping all indexes from the original dataframe. However, this process is slow and as the iterations and number of items in the dataframe increase the speed becomes more of an issue.
import pandas as pd
import numpy as np
import timeit

data_len = 1000
half = int(data_len/2)
fifth = int(data_len/5)
ans = ['Cat'] * data_len
cols = ['White'] * half + ['Black'] * half
age = np.random.randint(1, 10, data_len)
breed = ['A'] * fifth + ['B'] * fifth + ['C'] * fifth + ['D'] * fifth + ['E'] * fifth
weights = np.random.uniform(1, 100, data_len)
heights = np.random.uniform(1, 100, data_len)

df = pd.DataFrame(columns=['Animal', 'Color', 'Age', 'Breed', 'Weight', 'Height'])

df['Animal'] = ans
df['Color'] = cols
df['Age'] = age
df['Breed'] = breed
df['Weight'] = weights
df['Height'] = heights

def get_largest(df):
    drop_id = []
    sort_cols = ['Weight', 'Height']
    for Animal in pd.unique(df['Animal']):
        animals_df = df.loc[np.in1d(df['Animal'], Animal)]
        for Color in pd.unique(animals_df['Color']):
            color_df = animals_df.loc[np.in1d(animals_df['Color'], Color)]
            for uid in pd.unique(color_df['Age']):
                age_df = color_df.loc[np.in1d(color_df['Age'], uid)]
                for vic in pd.unique(age_df['Breed']):
                    breed_df = age_df.loc[np.in1d(age_df['Breed'], vic)]
                    drop_id.extend(
                        breed_df.sort_values(by=sort_cols, ascending=[False] * len(sort_cols)).index[1:].values)
    df = df.drop(df.index[drop_id])
    return df

func = lambda: get_largest(df)
print(timeit.timeit(func, number=100))

I may have misunderstood what your are trying to achieve. However it sounds like you want to filter your dataframe for a unique list animal, color, age and breed. and from that take that largest, weight and height.
you could achieve this by sorting your data frame by weight then height this will sort from lowest to highest.
you can then drop duplicates based on the animal, color, age and breed and specify to the drop method to keep the last row(This will be the biggest since the table is ordered from low to high).
We can write this in a general function that will accept three parameters, a df, a list to sort by and a list to filter by.
def my_df_filter(df: pd.DataFrame, sort_by: List[str], drop_by: List[str]) -> pd.DataFrame:
    sorted_df = df.sort_values(by=sort_by)
    dropped_df = sorted_df.drop_duplicates(subset=drop_by, keep='last')
    return dropped_df

This could actually all be returned in a single line by chaining them together. However I have broken it down for more readability. But you could write as
def my_df_filter1(df: pd.DataFrame, sort_by: List[str], drop_by: List[str]) -> pd.DataFrame:
    return df.sort_values(by=sort_by).drop_duplicates(subset=drop_by, keep='last')

using your original code and a dataset of 100,000 rows. it reduced it from 100k rows to 54 rows and running it 100 times via timeit on my laptop took around 18 seconds.
import pandas as pd
import numpy as np
import timeit
from typing import List

def my_df_filter(df: pd.DataFrame, sort_by: List[str], drop_by: List[str]) -> pd.DataFrame:
    return df.sort_values(by=sort_by).drop_duplicates(subset=drop_by, keep='last')

data_len = 100000
half = int(data_len / 2)
fifth = int(data_len / 5)
data = {
    'Animal': ['Cat'] * data_len,
    'Color': ['White'] * half + ['Black'] * half,
    'Age': np.random.randint(1, 10, data_len),
    'Breed': ['A'] * fifth + ['B'] * fifth + ['C'] * fifth + ['D'] * fifth + ['E'] * fifth,
    'Weight': np.random.uniform(1, 100, data_len),
    'Height': np.random.uniform(1, 100, data_len)
}
df = pd.DataFrame(data)

print(df.shape)
print(my_df_filter(df, sort_by=['Weight', 'Height'], drop_by=['Animal', 'Color', 'Age', 'Breed']).shape)

func = lambda: my_df_filter(df, sort_by=['Weight', 'Height'], drop_by=['Animal', 'Color', 'Age', 'Breed'])
print(timeit.timeit(func, number=100))

OUTPUT
(100000, 6)
(54, 6)
18.2784114

Having a general function means that its much easier to increase or reduce the number of columns to filter or sort by without having to change much code.


Distributing points uniformly in a unit circle I have been working on a code to distribute randomly generated points uniformly in a circle of radius 1 centered at origin. I have tried two variations of the code, yet they both yield one major problem.

Even for as many as 500 points, there is a particular region (quadrant 1, (0,1) on X Axis, and about (0,0.5) on Y axis) which is literally empty and it seems like that is a gaping hole in an otherwise perfectly well distributed points in the circle (the points seems to be distributed uniformly in the region outside this). The following are the two codes I tried out:

#include<iostream>
#include<random>
#include<time.h>
#include<fstream>
#define pi atan(1)*4
using namespace std;
int main()
{
    int N;  cin>>N;
    double X,Y,x,y,a,b;
    srand(time(NULL));
    for(int i=0; i<N; i++)
    {
        do
            {   X=(double)rand()/(double)RAND_MAX;
                Y=(double)rand()/(double)RAND_MAX;
                x=sqrt(X);
                y = 2*(pi)*Y;
                a=x*cos(y);
                b=x*sin(y);
            }while(((a*a)+(b*b))<=1);
    }
    return 0;
}

The second code is:

#include<iostream>
#include<random>
#include<time.h>
#include<fstream>
#define pi atan(1)*4
using namespace std;
int main()
{
    int N;  cin>>N;
    double X,Y,x,y,a,b;
    random_device R;
    mt19937 G(R());
    uniform_real_distribution<double> D(0,1);
    for(int i=0; i<N; i++)
    {
        do
            {
                X=D(G);     Y=D(G);     //uniform real distribution in (0,1)
                x=sqrt(X);
                y = 2*(pi)*Y;
                a=x*cos(y);
                b=x*sin(y);
            }while(((a*a)+(b*b))<=1);

    }
    return 0;
}

What is the possible reason for the problem I'm facing?

Update: For some reason, all points started getting generated at the boundary if I use ((aa)+(bb)<1) in the do-while loops. The problem I mentioned earlier has ((x * x)+(y * y)<1) which is clearly wrong, my bad.
I replaced the do-while of ((a * a)+(b * b)<1) with a goto as follows

A:
X=D(G);     Y=D(G);     //uniform real distribution in (0,1)
x=sqrt(X);
y = 2*(pi)*Y;
a=x*cos(y);
b=x*sin(y);
if((a*a)+(b*b)>1)
{  gota A:   }

and now it seems to be working fine. What could be the reason that goto makes it work but do-while does not?

You can paramterize a circle via a radius and the angle. Therefore, for the  cycle with radius r around (h,w) you can calculate the x and y coordinates with the following equations

x = h + r * sin(theta)
y = w + r *cos(theta)

Therefore your code should look like that
random_device R;
mt19937 G(R());
uniform_real_distribution<double> D(0,360);

for(int i=0; i<N; i++) {
    angle = D(G);
    x = sin(angle);
    y = cos(angle);
}

UPDATE
Toby is right, that this only adds points to the unit circle, rather than the full area of the circle. The solution is rather simple, add a random variable for the radius
unsigned seed = std::chrono::system_clock::now().time_since_epoch().count();
mt19937 rngAngle(seed);
seed = std::chrono::system_clock::now().time_since_epoch().count();
mt19937 rngRadius(seed);
uniform_real_distribution<double> angleDistribution(0,360);
uniform_real_distribution<double> radiusDistribution(0,1);

for(int i=0; i<N; i++) {
    angle  = angleDistribution(rngAngle);
    radius = radiusDistribution(rngRadius);
    x = radius * sin(angle);
    y = radius * cos(angle);
}

As you have seen, I used some better names too and seeded both random number generators independently with the current time.


Shuffling sections of an ordered list based on tuple values I have a list as follows and I  want to shuffle the elements that have the same frequency while keeping the elements with different frequencies in the correct positions. The code below works; however, it is not that nice. I was wondering if anyone had some suggestions e.g. using list comprehension.

pieces_needed = [('a',1),('b',1),('d',1),('c',2),('g',3),('f',5),('z',5)]
temp = []
new_pieces_needed = []
current_freq = 1
for i in pieces_needed:
   if i[1] == current_freq:
      temp.append(i)
   else:
      random.shuffle(temp)
      new_pieces_needed +=  temp
      temp = []
      temp.append(i)
      current_freq = i[1]

random.shuffle(temp)
new_pieces_needed +=  temp

print(new_pieces_needed)

Example output:

[('d',1),('b',1),('a',1),('c',2),('g',3),('z',5),('f',5)]

Both branches of the if statement have the line:

temp.append(i)

This duplication could be avoided.
The variable temp could have a better name, for example group or current_group.
i[1] is evaluated twice.
The variable name i is normally used for an index, but in this case it is an element of the list, so it could have a better name.
These lines appear twice:

random.shuffle(temp)
new_pieces_needed +=  temp

This duplication could be avoided by postponing the shuffles until the groups have been found, like this:

# Group pieces_needed according to their frequency.
groups = []
current_freq = object()
for piece in pieces_needed:
    freq = piece[1]
    if freq != current_freq:
        groups.append([])
        current_freq = freq
    groups[-1].append(piece)

# Randomly shuffle each group.
for group in groups:
    random.shuffle(group)

# Concatenate the shuffled groups.
new_pieces_needed = [piece for group in groups for piece in group]

There is something arbitrary about the use of "pieces" and "frequency" and piece[1]. The same general algorithm would work for any kind of data, not just pieces. So I think the code would be clearer if we generalized it like this, taking an arbitrary iterable and an optional key function:

def shuffle_groups(iterable, key=None):
    """Group the iterable (using the key function if supplied), randomly
    shuffle each group, and return the concatenation of the shuffled
    groups.

    """

and then we can shuffle the pieces by passing operator.itemgetter(1) for the key argument:

from operator import itemgetter
new_pieces_needed = shuffle_groups(pieces_needed, key=itemgetter(1))

The grouping logic can be delegated to itertools.groupby and the concatenation to itertools.chain:

from itertools import chain, groupby
from random import shuffle

def shuffle_groups(iterable, key=None):
    """Group the iterable (using the key function if supplied), randomly
    shuffle each group, and return the concatenation of the shuffled
    groups.

    """
    groups = [list(group) for _, group in groupby(iterable, key)]
    for group in groups:
        shuffle(group)
    return list(chain(*groups))


C++ function composition What is a good way to compose std::function objects in C++?

I tried the following, and it seems to work well:

template<typename ... Fs>
struct compose_impl
{
    compose_impl(Fs&& ... fs) : functionTuple(std::forward_as_tuple(fs ...)) {}

    template<std::size_t> struct int2type{};

    template<size_t N, typename ... Ts>
    auto apply(int2type<N>, Ts&& ... ts)
    {
        return std::get<N>(functionTuple)(apply(int2type<N+1>(),std::forward<Ts>(ts)...));
    }

    static const size_t size = sizeof ... (Fs);
    template<typename ... Ts>
    auto apply(int2type<size-1>, Ts&& ... ts)
    {
        return std::get<size-1>(functionTuple)(std::forward<Ts>(ts)...);
    }

    template<typename ... Ts>
    auto operator()(Ts&& ... ts)
    {
        return apply(int2type<0>(), std::forward<Ts>(ts)...);
    }

    std::tuple<Fs ...> functionTuple;
};

template<typename ... Fs>
auto compose(Fs&& ... fs)
{
    return compose_impl<Fs ...>(std::forward<Fs>(fs) ...);
}

With this, one can compose functions as long as the signatures fit together. Example:

auto f1 = [](std::pair<double,double> p) {return p.first + p.second; };
auto f2 = [](double x) {return std::make_pair(x, x + 1.0); };
auto f3 = [](double x, double y) {return x*y; };
auto g = compose(f1, f2, f3);

std::cout << g(2.0, 3.0) << std::endl;   //prints '13', evaluated as (2*3) + ((2*3)+1)

Comments and suggestions for improvement are welcome!

Generally speaking, it is really well done, for several reasons: std::tuple often takes advantage of the empty base class optimization, which means that since you feed it lambdas, your class will often weigh almost nothing, and everything is correctly forwarded. The only things I see that could be improved are the following ones:

You could const-qualify apply and operator().
size should be static constexpr instead of static const to make it even clearer that it is a compile-time constant.
You should be consistent when qualifying std::size_t: either use the prefix std:: or leave it, but stay consistent.

As you can see, these are really minor improvements. I also have some other remarks, but those will be opinions more than actual advice:

int2type kind of already exists in the standard and is named std::integral_constant. However, I will concede that it takes another template parameter for the type and that it might be too verbose for your needs.
I had some trouble understanding how your recursion worked because it was in ascending order. For some reason, I am more used to descending order. I would have overloaded apply for int2type<0> and not for int2type<size-1> and performed a descending recursion. That would have allowed me to write:

template<typename ... Ts>
auto operator()(Ts&& ... ts)
{
    return apply(int2type<sizeof ... (Fs) - 1>(), std::forward<Ts>(ts)...);
}

And then, size wouldn't have had to be a member of the class anymore. But I have to admit that this is an opinion and not a guideline. Your code is good enough that I see almost nothing that could be improved :)


Finding perimeters of right triangles with integer-length legs, up to a limit I have a nested for-loop that populates a list with elements:

a = []
for i in range(1, limit+1):
    for j in range(1, limit+1):
        p = i + j + (i**2 + j**2)**0.5
        if p <= limit:
            a.append(p)

I could refactor it into list comprehension:

a = [i + j + (i**2 + j**2)**0.5
     for i in range(1, limit+1)
     for j in range(1, limit+1)
     if i + j + (i**2 + j**2)**0.5 <= limit]

But now the same complex expression is in both parts of it, which is unacceptable. Is there any way to create a list in a functional way, but more elegantly?

I guess in Lisp I would use recursion with let. How it is done in more functional languages like Clojure, Scala, Haskell?

In Racket it's possible to bind expressions inside a for/list comprehension. I've found one solution to my problem:

[k
 for i in range(1, limit+1)
 for j in range(1, limit+1)
 for k in [i + j + (i**2 + j**2)**0.5]
 k <= limit]

I'm not sure how pythonic it is.

You can rewrite it with two separate comprehensions.  One to generate the values and one to filter them out.  You can use itertools.product to get the cartesian product.

a = [x for x in (i + j + (i**2 + j**2)**0.5
                 for i, j in itertools.product(range(1, limit+1), repeat=2))
     if x <= limit]


Python functions to determine if a given ticker symbol is an ETF I am working on a stock analysis application in Python and I have the need to determine whether an input ticker symbol is an Exchange Traded Fund (ETF) or not.  First I need to determine if the given ticker is even valid.  Second I need to determine if it is an ETF or a company stock.  So, I have some simple functions to request info from Yahoo! Finance (modified from ystockquote)

from urllib.request import Request, urlopen

def _yahoo_request(symbol, tag):
    """
    Retrieve stock data from Yahoo! Finance.

    Args:
        symbol (str): Stock ticker symbol for company of interest.
        tag (str): Yahoo! finance tag for a given property.

    Returns:
        str: The property requested.
    """
    url = 'http://finance.yahoo.com/d/quotes.csv?s=%s&f=%s' % (symbol, tag)
    request = Request(url)
    response = urlopen(request)
    content = response.read().decode().strip().strip('"')
    return content

So to determine if a stock is valid, I check to see if it's name exists in Yahoo! Finance.  Getting the name:

def get_name(symbol):
    """
    Retrieve company name for given ticker.

    Args:
        symbol (str): Ticker symbol for a given company's stock.

    Returns:
        str: The company name. Returns 'N/A' if ticker not in Yahoo! Finance.
    """
    return _yahoo_request(symbol, 'n')

and checking if it is a valid ticker:

def valid_ticker(ticker):
    """
    Check whether given ticker is a valid stock symbol.

    NOTE: Assumes that a stock is valid IF Yahoo! Finance returns a
    name for the given ticker.

    Args:
        ticker (str): Ticker symbol in question.

    Returns:
        bool: True if valid ticker (returned name != 'N/A'),
            otherwise False.
    """
    ticker = ticker.upper()
    return True if get_name(ticker) != 'N/A' else False

Finally, to determine if a given ticker is for an ETF or not, I will check the number of shares outstanding.

def get_shares_outstanding(symbol):
    """
    Retrieve number of shares outstanding for given stock ticker.

    Args:
        symbol (str): Ticker symbol for a given company's stock.

    Returns:
        str: The company name.
    """
    return _yahoo_request(symbol, 'j2')

This is not a valid data point for ETF's, so again, Yahoo! will return 'N/A'.  So, if the response can be cast as an integer, the ticker is not an ETF.

def is_etf(ticker):
    """
    Determine whether a given ticker symbol is for an Exchange
    Traded Fund (ETF).

    NOTE: Assumes that ticker is an ETF IF it is a valid ticker AND
    Yahoo! Finance does not return a numeric value for 'shares
    outstanding.'

    Args:
        ticker (str): Ticker symbol in question.

    Returns:
        bool: None if not a valid ticker, True if an etf (ticker is
        valid and returned shares outstanding cannot be converted to
        an int), otherwise False.
    """
    ticker = ticker.upper()
    if valid_ticker(ticker):
        try:
            int(get_shares_outstanding(ticker))
            return False
        except ValueError:
            return True
    else:
        return None

Code is working as expected. Does this seem a reasonable way to check this? In particular the last function.  Should I just use if get_shares_outstanding(ticker) == 'N/A': return True ?

Any time you're dealing with structured data, use a proper parser for it.

url = 'http://finance.yahoo.com/d/quotes.csv?s=%s&f=%s' % (symbol, tag)

In this case, you want urllib.parse.

Please use a context manager for:

request = Request(url)
response = urlopen(request)

eg.

with urlopen(Request(url)) as response:
    ...

Context managers are good. It's a shame they're so under-used. On older Pythons, you should use contextlib.closing instead.

I would generally merge these lines into a long return:

content = response.read().decode().strip().strip('"')
return content

Consider that in

ticker = ticker.upper()
return True if get_name(ticker) != 'N/A' else False

True if some_bool else False is just some_bool, so write

return get_name(ticker.upper()) != 'N/A'

instead.


Recursively moving every lowercase 'x' in a string to the end Given a string, compute recursively a new string where all the
  lowercase 'x' chars have been moved to the end of the string.

endX("xxre") → "rexx"
endX("xxhixx") → "hixxxx"
endX("xhixhix") → "hihixxx"

Here's the initial idea I've had:

public String endX(String str) {
  ArrayList<Character> track = new ArrayList<Character>();
  return end(str,"",0,track);
}

public String end(String str,String res,int index,ArrayList<Character> track)
{
  if(index==str.length() && track.size()==0)
  {
    return res;
  }
  else if(index==str.length() && track.size()!=0)
  {

    return end(str,res+String.valueOf(track.remove(0)),index,track);
  }
  else if(str.charAt(index)=='x')
  {
    track.add(str.charAt(index));
    return end(str,res,index+1,track);
  }
  else
  {
    return end(str,res+str.charAt(index),index+1,track);
  }

}

I insisted on now using Java substring builtin functions since its complexity is \$O(n)\$. The above solution takes \$O(n)\$ time and \$O(n)+O(n)\$ space complexity.

How I can I optimize my solution further in terms of space and time complexity and total number of variables?

Your solution takes \$O(\frac{(n*n)}{2} + \frac{n}{2})\$ space and \$O(\frac{(n*n)}{2} + \frac{n}{2})\$ time, where \$n\$ is the amount of characters.

The flaw is trifold:

You make use of recursion. If the compiler doesn't optimize the recursion away, you keep the entire string in memory for each stackframe. This explains why there's a * in the O for space.

Strings are immutable, so when you perform operations on them, you get back a new copy.

When a new copy of a string is made, it has to copy every character in that string to another location in memory. This means you have to copy the entire res variable every time you add to it.

What this all means is that each time you call end with res+str.charAt(index) or res+String.valueOf(track.remove(0)), you have two strings in memory:

First call to end, with the empty res
Second call to end with a res of length 1

So if the input string is "xxhixx":

The first time you add an x to track
The second time you add an x to track
The third time you add an h to res, putting a new string of length 1 in memory
The fourth time you add an i to res, putting a new string of length 2 in memory
The fifth time you add an x to track
The sixth time you add an x to track, then the input string is done
The seventh time you remove an x from track and add it to res, putting a new string of length 3 in memory
The eighth time you remove an x from track and add it to res, putting a new string of length 4 in memory
The ninth time you remove an x from track and add it to res, putting a new string of length 5 in memory
The tenth time you remove an x from track and add it to res, putting a new string of length 6 in memory
The eleventh time you enter end, both track and str have been processed, and you return res

If we look at the memory usage when you enter end for the eleventh time, you'll see that there are 6 copies of res:

"h"
"hi"
"hix"
"hixx"
"hixxx"
"hixxxx"

That makes the total memory usage \$6+5+4+3+2+1 = 21 \space \text{characters}\$, consistent with \$\frac{(n*n)}{2} + \frac{n}{2}\$. \$6*6 = 36\$, \$\frac{36}{2} = 18\$, \$\frac{6}{2} = 3\$, \$18+3 = 21\$.

If the compiler notices that it can optimize your recursion away, you will not have this issue - the space complexity will be \$O(n)\$. However, internally, it won't be recursion anymore.

Since you're forced to use recursion, what you have is close to the best you can do.

You could convert track to a String - that way, you can just append it to res in one go when you're done.

But we don't like arbitrary requirements, so let's fix the algorithm.

It's pretty simple:

public String endX(String inputString) {
    int xCounter = 0;
    int writeIndex = 0;
    final int size = inputString.length();
    char[] resultArray = new char[size];
    for (int readIndex = 0; readIndex < size; readIndex++) {
        char c = inputString.charAt(readIndex);
        if(c == 'x'){
            xxCounter++;
            resultArray[size - xCounter] = c;
        } else {
            resultArray[writeIndex++] = c;
        }
    }
    return new String(resultArray);
}

It's \$O(2n)\$ in terms of space (1x for resultArray, 1x for the new String). It's \$O(2n)\$ in terms of time (1x for going through the string, another 1x for the new String because it's a constructor).

I'll explain in detail:

We create a character array to store the new string. We then check each character of the input string. If the character is a normal character, write it into the character array and move on to the next. If it's an X, write it at the end of the array (size - 1). The next time you encounter an X, write it at the location 1 before the end of the array (size - 2). And so on.

Lastly, convert to String via the character array constructor.


A shim to make make Node GRPC bindings more JavaScript-y This function allows me to write this code

const audioOutConfig = new AudioOutConfig();
audioOutConfig.setSampleRateHertz(16000);
audioOutConfig.setEncoding(AudioOutConfig.Encoding.LINEAR16);
audioOutConfig.setVolumePercentage(100);

as this code

const audioOutConfig = config(new AudioOutConfig(), {
  sampleRateHertz: 16000,
  encoding: AudioOutConfig.Encoding.LINEAR16,
  volumePercentage: 100
});

Is this a bad idea, and is the function I wrote to do it acceptable?

function config(obj, opts) {
  for (var opt in opts) {
    if (opts.hasOwnProperty(opt)) {
      obj[`set${opt.charAt(0).toUpperCase() + opt.slice(1)}`](opts[opt]);
    }
  }
  return obj;
}

Both the idea and implementation look sound to me. As a nitpick, you might consider using a reduce on the opts, giving you a one liner. Also, I believe the hasOwnProperty test is superfluous here

Untested reduce version

Note that, per Joseph's suggestion, which I agree with, this creates the AudioOutConfig object for you, rather than requiring it as a parameter:

function config(opts) {
  return Object.keys(opts).reduce((audio, opt) => {
    const methodName = `set${opt.charAt(0).toUpperCase() + opt.slice(1)}`
    return audio[methodName] ?  audio[methodName](opts[opt]) : audio
  }, new AudioOutConfig())
}


SQL String to get data from DB2 I decided to split the SQL Part away from This Question.
Before we get started I need to let you know a critical piece of information: Due to permissions within an offsite database I am NOT allowed to create tables even temporary ones within the database that I am getting the data from.

With that being said: All of the code below works as expected, but I would like a review of it because I know that there has to be a cleaner way of writing the SQL String.

SELECT CONCAT(cfcif#,cfalta) AS Customer_Number,
                 cffna AS First_Name,
                 cfmna AS Middle_Name,
                 COALESCE(NULLIF(cflna,''),cfna1) AS Last_Name,
                 COALESCE(NULLIF(RTRIM(LTRIM(cfpfa1))|| ' '|| RTRIM(LTRIM(cfpfa2)),''),RTRIM(LTRIM(cfna2),'')|| ' ' || RTRIM(LTRIM(cfna3),'')) AS Street_Address,
                 COALESCE(NULLIF(cfpfcy,''),cfcity) AS Street_City,
                 COALESCE(NULLIF(cfpfst,''),cfstat) AS Street_State,
                 COALESCE(NULLIF(LEFT(cfpfzc, 5), 0), LEFT(cfzip, 5)) AS Street_Zip,
                 CONCAT(RTRIM(LTRIM(cfna2)),RTRIM(LTRIM(cfna3))) AS Mailing_Address,
                 cfcity AS Mailing_City,
                 cfstat AS Mailing_State,
                 LEFT(cfzip, 5) AS Mailing_Zip,
                 NULLIF(cfhpho,0) AS Home_Phone,
                 NULLIF(cfbpho,0) AS Business_Phone,
                 NULLIF(cfssno,0) AS TIN,
                 (CASE
                    WHEN cfindi = 'Y' THEN '1'
                    WHEN cfindi = 'N' THEN '2'
                   END) AS Customer_Type,
                 (CASE
                    WHEN cfdob7 = 0 THEN NULL
                    WHEN cfdob7 = 1800001 THEN NULL
                    ELSE cfdob7
                  END) AS Date_of_Birth,
                 cfeml1 AS Email_Address
    FROM bhschlp8.jhadat842.cfmast cfmast
    WHERE cfdead = 'N'
    ORDER BY cfcif#

I can't see much wrong with it.  Maybe use BTRIM if on Db2 11.1 or above rather than RTRIM(LTRIM(
and maybe indent with fewer spaces (e.g. align the first column with the rest), but that is really a matter of style


Python grouped iterator I wanted to write an iterator that would split items up into groups, so i can bulk insert into mysql without crashing or overwhelming the server.

This is the implementation I came up with.
I am hoping that when yielding from the list its not creating a copy / clone of the list.
I need more experience with python internals to be able to debug / performance profile this method i wrote.

def iter_group(iterable, batch_size:int):
    iterable_type = type(iterable)
    length = len(iterable)
    start = batch_size*-1
    end = 0
    while(end < length):
        start += batch_size
        end += batch_size
        if iterable_type == list:
            yield (iterable[i] for i in range(start,min(length-1,end)))
        else:
            yield iterable[start:end]

#test data

items = range(0,10001)
for item_group in iter_group(items, 1000):
    for item in item_group:
        print(item,end=' ')

We can improve readability of your code as follows:

using type annotations partially looks incomplete, when we can be more descriptive:

from typing import (Iterable,
                    TypeVar)

ElementType = TypeVar('ElementType')

def iter_group(iterable: Iterable[ElementType],
               batch_size: int) -> Iterable[Iterable[ElementType]]:
    ...

checking if object has given type should be done using isinstance built-in function, so instead of

iterable_type = type(iterable)
...
if iterable_type == list:
    ...

we can simply write

if isinstance(iterable, list):
    ...

(btw I don't understand why are you treating list as a special case)
if you want negate a number then we can use - unary operator, no need to multiply by -1, so instead of

start = batch_size * -1

we can write

start = -batch_size

no need in parentheses for while-loop condition, we are not in C/C++/JAVA, we can simply write:

while end < length:
    ...

setting start to -batch_size and end to 0 and change them right after that seems redundant when we can set them to 0 and batch_size respectfully and increment at the end of the while loop body, so instead

start = batch_size * -1
end = 0
while end < length:
    start += batch_size
    end += batch_size
    ...

we can write

start = 0
end = batch_size
while end < length:
    ...
    start += batch_size
    end += batch_size

If I understood correctly what you are trying to achieve is to split iterable into evenly sized chunks, which is a well-known StackOverflow question, but accepted answer works only for sequences (str/list/tuple) and so does your solution. For example it won't work for potentially infinite sequences like classic Fibonacci numbers generator

>>> def fibonacci():
        a, b = 0, 1
        while True:
            yield a
            a, b = b, a + b
>>> next(iter_group(fibonacci(), 5))
Traceback (most recent call last):
    ...
        length = len(iterable)
TypeError: object of type 'generator' has no len()

If you want to work with arbitrary iterables (e.g. generators), we may use itertools module and this brilliant solution which uses iter built-in function form with sentinel value

from itertools import islice
from typing import (Iterable,
                    Tuple,
                    TypeVar)

ElementType = TypeVar('ElementType')

def chunks(iterable: Iterable[ElementType],
           batch_size: int) -> Iterable[Tuple[ElementType, ...]]:
    iterator = iter(iterable)
    return iter(lambda: tuple(islice(iterator, batch_size)), ())

but if we make a simple benchmark with

def sequence_chunks(iterable: Sequence[ElementType],
                    batch_size: int) -> Iterable[Sequence[ElementType]]:
    for start in range(0, len(iterable), batch_size):
        yield iterable[start:start + batch_size]

like

import timeit

...
print('original solution',
      min(timeit.repeat('list(iter_group(iterable, 1000))',
                        'from __main__ import iter_group\n'
                        'iterable = range(0, 10001)',
                        number=10000)))
print('Ned Batchelder\'s solution',
      min(timeit.repeat('list(sequence_chunks(iterable, 1000))',
                        'from __main__ import sequence_chunks\n'
                        'iterable = range(0, 10001)',
                        number=10000)))
print('senderle\'s solution',
      min(timeit.repeat('list(chunks(iterable, 1000))',
                        'from __main__ import chunks\n'
                        'iterable = range(0, 10001)',
                        number=10000)))

on my laptop with Windows 10 and Python 3.5 we'll have

original solution 0.07320549999999999
Ned Batchelder's solution 0.06249870000000002
senderle's solution 2.6072023999999994

so how can we have both speed and handle cases with non-sequence iterables?

Here comes functools module with singledispatch function decorator. We can use it like

import timeit
from collections import abc
from functools import singledispatch
from itertools import islice
from typing import (Iterable,
                    Sequence,
                    TypeVar)

ElementType = TypeVar('ElementType')

@singledispatch
def chunks(iterable: Iterable[ElementType],
           batch_size: int) -> Iterable[Iterable[ElementType]]:
    iterator = iter(iterable)
    return iter(lambda: tuple(islice(iterator, batch_size)), ())

@chunks.register(abc.Sequence)
def sequence_chunks(iterable: Sequence[ElementType],
                    batch_size: int) -> Iterable[Iterable[ElementType]]:
    for start in range(0, len(iterable), batch_size):
        yield iterable[start:start + batch_size]

after that call to chunks will end up in sequence_chunks for sequences and in general chunks for all other cases.

But

print('single-dispatched solution',
      min(timeit.repeat('list(chunks(iterable, 1000))',
                        'from __main__ import chunks\n'
                        'iterable = range(0, 10001)',
                        number=10000)))

gives

single-dispatched solution 0.0737681

so we lose some time during dispatching, but we can save some time and space if we will use itertools.islice for sequences as well like

@chunks.register(abc.Sequence)
def sequence_chunks(iterable: Sequence[ElementType],
                    batch_size: int) -> Iterable[Iterable[ElementType]]:
    iterator = iter(iterable)
    for _ in range(ceil_division(len(iterable), batch_size)):
        yield islice(iterator, batch_size)

def ceil_division(left_number: int, right_number: int) -> int:
    """
    Divides given numbers with ceiling.
    """
    # based on https://stackoverflow.com/a/17511341/5997596
    return -(-left_number // right_number)

which gives

complete single-dispatched solution 0.03895900000000002

P.S.

@AJNeufeld's solution gives

0.0647120000000001

on my laptop


Checking that all occurrences of 'g' have another 'g' as a neighbor We'll say that a lowercase 'g' in a string is "happy" if there is another 'g' immediately to its left or right. Return true if all the g's in the given string are happy.

gHappy("xxggxx") → true
gHappy("xxgxx") → false
gHappy("xxggyygxx") → false

This is my code so far, which I think is very messy.

  public boolean gHappy(String str)
  {
        for(int i=1;i<str.length()-1;i++)
           {
              if(str.charAt(i)=='g')
                 if(str.charAt(i-1)!='g'&&str.charAt(i+1)!='g')
                    return false;
  }
  if(str.length()==1)
      return false;
  if(str.equals(""))
      return true;
if(str.charAt(str.length()-1)=='g')
      if(str.charAt(str.length()-2)!='g')
        return false;

  return true;
}

Other answers have good input on a number of factors I agree with, especially about the early-return logic, the bugs that were pointed out, and the bracing strategies.

My suggestion though, is to use a different (the right?) tool for the job.

Regular expressions are a tool that are designed for text processing, and a well-crafted expression is typically faster and more concise than the hand-coded alternative. Regular expressions are not always the solution, but in this case, the result would be simpler to read (for someone familiar with them).

In this case, the logic requires checking that all 'g' chars are happy for the check to return true. Alternatively, to make sure there are no unhappy 'g' chars.

The expression to check for unhappy 'g' chars will be:

 private static final Pattern UNHAPPYG = Pattern.compile("(?<!g)g(?!g)");

That expression says "Match any 'g' that has no 'g' before it, and no 'g' after it.

Then, you can use it as:

private static final boolean gHappy(String str) {
    return !UNHAPPYG.matcher(str).find();
}

I have put together an Ideone example here.


Project Euler #2 in Python Here is my Python code for Project Euler #2:

  Each new term in the Fibonacci sequence is generated by adding the
  previous two terms. By starting with 1 and 2, the first 10 terms will
  be:

  1, 2, 3, 5, 8, 13, 21, 34, 55, 89, ...

  By considering the terms in the Fibonacci sequence whose values do not
  exceed four million, find the sum of the even-valued terms.

How well did I do and what improvements can I make?

sum = 0
fib = []
i = 2

fib.append(0), fib.append(1), fib.append(1)
while fib[i] < 4000000:
    i += 1
    fib.append(fib[i-1] + fib[i-2])
    if (fib[i] % 2 ==0):
        sum += fib[i]

print sum

EDIT: Thanks everyone for answering and commenting. All answers and some comments provide different suggestions which is very helpful.

Your use of an array to store previous entries is convenient, but it requires a lot of memory use for values that will be thrown away and not reused. It's very simple to keep the two most recent values in simple variables, and not have the array at all.

Consider:

sum = 0
current = 1
previous = 1

while True:
    fib = current + previous
    if (fib > 4000000):
        break

    previous = current
    current = fib

    if (fib % 2 == 0):
        sum += fib

print sum

By eliminating the array you save a lot of management time, and space.

The code above contains a break condition inside the infinite loop instead of a loop condition. This is a common practice solution in Python for a do-while loop instead of a while-do loop.


Check if num X is divisible by num Y I wrote some code for a course:

  Create a program that takes two numbers from the user then find out if the first is divisible by the second. If not divisible, let the user know what the remainder is.

Here is what I currently have. Is this right? If so, how could I improve it?

puts "Enter number one"
num1 = gets.chomp.to_i

puts "Enter number two"
num2 = gets.chomp.to_i

if num1 % num2 != 0
    p num1 % num2
else p "The numbers are divislbe."
end

Calculate the remainder once. Right now you calculate it to the comparison, and again if you need to output it.
Use .zero? for readable, idiomatic code
Fix your linebreaks and indentation (don't put code on the same line as else
Consider what to do if the divisor is zero.
Optionally, extract the bit of logic that gets a number into a method

This is a (intentionally overwrought) example:

def get_int(message)
  puts message
  gets.chomp.to_i
end

numerator = get_int("Enter numerator")
divisor = get_int("Enter divisor")

if divisor.zero?
  puts "Can't divide by zero"
  exit
end

remainder = numerator % divisor

if remainder.zero?
  puts "#{numerator} is divisble by #{divisor}"
else
  puts "#{numerator} is not divisble by #{divisor}. Remainder is #{remainder}"
end


Checking an MD5 of a device The point of this code snippet is to calculate and compare an MD5 obtained from a USB device connected to the computer against a database to check its validity.

var md5String = string.Empty;
using (var md5 = MD5.Create())
{
    var md5CRC = md5.ComputeHash(File.ReadAllBytes(destinationFilename));
    foreach (byte b in md5CRC)
        md5String += b.ToString("X2");
}

// here I check the MD5 against the database to check the usb device is valid

The code meets its purpose, but I want to know how I can improve this code.

Using string concatenation in a loop is a no go because each time you do a += a new string object will be created because strings are immutable.

To use the right tool for the job I would suggest to use a StringBuilder instead. Using a StringBuilder doesn't have this restriction because the passed strings are just appended char by char to the internal buffer.

Using braces {} although they might be optional will make your code less error prone.

By extracting the building of the hex string representation to a separate method the responsibility is split nicely and could be reused if needed. Having this as a extension method could serve well.

private string ToHex(this byte[] values)
{
    if (values == null) { throw new ArgumentNullException("values"); }

    StringBuilder sb = new StringBuilder(values.Length * 2);
    foreach (var b in values)
    {
        sb.Append(b.ToString("X2"));
    }

    return sb.ToString();
}

resulting in the former code like this

var md5String = string.Empty;
using (var md5 = MD5.Create())
{
    var md5CRC = md5.ComputeHash(File.ReadAllBytes(destinationFilename));

    md5String = md5CRC.ToHex();
}


Game of Noughts and Crosses Feedback this is my 2nd project on Python. I created a game of Noughts and Crosses(Tic-Tac-Toe) that you can play against the computer. What are your thoughts on my implementation of the game? Is there anything I've missed or could improve upon?
I did think about adding a figure window and making it more interactive by clicking on where you want to move however that seemed a bit too advanced.
import random
import sys
import numpy as np

# Today we are going to make an interactive game in Python.
# This is going to be a game of noughts and crosses!

#  Initialise grid and collection of X's or O's needed to win a game.
grid = [["_", "_", "_"], ["_", "_", "_"], ["_", "_", "_"]]
print(np.asarray(grid))  # To make the output represent a grid instead of a list of lists.
row_os = ["O", "O", "O"]
row_xs = ["X", "X", "X"]
coords_picked = []

# Create a loop to find all co-ordinates in our grid
all_coords = []
for x in range(3):
    for y in range(3):
        all_coords.append((x, y))

turn = 0
remaining_coords = []

while turn < 10:
    # Find status of each column
    column_1 = [row[0] for row in grid]
    column_2 = [row[1] for row in grid]
    column_3 = [row[2] for row in grid]

    # Find status of 2 diagonals
    main_diag = [row[i] for i, row in enumerate(grid)]
    counter_diag = [row[-i - 1] for i, row in enumerate(grid)]

    # Winning conditions for each player
    if (grid[0] == row_os or grid[1] == row_os or grid[2] == row_os) or (
        column_1 == row_os or column_2 == row_os or column_3 == row_os) or (main_diag == row_os) or (
        counter_diag == row_os):
        print("The computer has won!")
        sys.exit()
    elif (grid[0] == row_xs or grid[1] == row_xs or grid[2] == row_xs) or (
        column_1 == row_xs or column_2 == row_xs or column_3 == row_xs) or (main_diag == row_xs) or (
        counter_diag == row_xs):
        print("Well done, you have won!")
        sys.exit()
    else:
        # If nobody has won yet, take another turn
        if turn % 2 == 0:
            x_coord = input()
            y_coord = input()
            coords_picked.append((int(x_coord), int(y_coord)))
            grid[int(x_coord)][int(y_coord)] = "X"
            print(np.asarray(grid))
            turn += 1
        else:
            # Computer randomly picks a co-ordinate in the grid that has not been picked yet
            remaining_coords = list(set(all_coords) - set(coords_picked))
            comp_coords = random.choice(remaining_coords)
            grid[comp_coords[0]][comp_coords[1]] = "O"
            coords_picked.append((comp_coords[0], comp_coords[1]))
            print(comp_coords)
            print(np.asarray(grid))
            turn += 1

print("A draw?! We must play again!")

I don't use numpy, and I'm getting errors when trying to use it on 3.9, but it seems unnecessary here. If you want to pretty-print a grid, that can be done succinctly using join:
def format_grid(grid):
    return "\n".join(" ".join(row) for row in grid)

Which, when you replace all uses of asarray, gives output like:
X _ _
_ _ _
_ _ O

I wouldn't import numpy just to help format. That's a heavy dependency for little gain.

These lines:
grid = [["_", "_", "_"], ["_", "_", "_"], ["_", "_", "_"]]
row_os = ["O", "O", "O"]
row_xs = ["X", "X", "X"]

Can make use of "sequence multiplication" to reduce the duplication:
SIDE_LENGTH = 3

grid = [["_"] * SIDE_LENGTH for _ in range(SIDE_LENGTH)]
row_os = ["O"] * SIDE_LENGTH
row_xs = ["X"] * SIDE_LENGTH

I'd change all_coords, coords_picked, and remaining_coords to be sets from the start. It isn't efficient to create them as lists, then convert them to sets whenever you want to use them. This unfortunately requires that remaining_coords be converted to a list before being given to random.choice, but that's still less conversions being done overall.
You could probably just get rid of remaining_coords too and just use:
comp_coords = random.choice(list(all_coords - coords_picked))

You have it as a global variable, even though you're recalculating it each time anyways. It would be even more efficient though to not recalculate it each time, and instead make a copy of all_coords at the start, then del elements from the set as they're picked. That's probably an unnecessary level of optimization for here though.

You can clean up the winning checks by making use of any and comprehensions. First, change column_1, column_2, and column_3 to:
columns = [[row[i]
           for row in grid]
           for i in range(SIDE_LENGTH)]

Whenever you're postfixing variable names with numbers, you probably want a list instead.
Next, the conditions themselves:
if any((any(row == row_os for row in grid),
        any(col == row_os for col in columns),
        any(diag == row_os for diag in [main_diag, counter_diag]))):
    print("The computer has won!")
    sys.exit()
elif any((any(row == row_xs for row in grid),
          any(col == row_xs for col in columns),
          any(diag == row_xs for diag in [main_diag, counter_diag]))):
    print("Well done, you have won!")
    sys.exit()

There's probably a better way of writing this still, but I haven't had my coffee yet. Basically, I'm again using a comprehensions (or rather, generator expressions) to reduce duplication. If you're writing grid[0], grid[1], grid[2] manually, you likely want to iterate over grid instead.
You could also wrap that condition up in a function, then use the function in both places since each condition is basically the same.
The inner anys could also be replaced with unpacking to make use of the outer any:
if any((*(row == row_os for row in grid),
        *(col == row_os for col in columns),
        *(diag == row_os for diag in [main_diag, counter_diag]))):

But I'm not sure that that's much better.

You should add in error handling. When testing this, I got multiple errors. I accidentally entered 3 in, and got an IndexError, and accidentally pressed enter without entering a number and got a ValueError. Really, the user entering bad input should be expected and handled.

At the end, I have:
import random
import sys

SIDE_LENGTH = 3

def format_grid(grid):
    return "\n".join(" ".join(row) for row in grid)

grid = [["_"] * SIDE_LENGTH for _ in range(SIDE_LENGTH)]
row_os = ["O"] * SIDE_LENGTH
row_xs = ["X"] * SIDE_LENGTH
coords_picked = set()

print(format_grid(grid))  # To make the output represent a grid instead of a list of lists.

# Create a loop to find all co-ordinates in our grid
all_coords = set()
for x in range(SIDE_LENGTH):
    for y in range(SIDE_LENGTH):
        all_coords.add((x, y))

turn = 0
remaining_coords = set()

while turn < 10:
    # Find status of each column
    columns = [[row[i]
               for row in grid]
               for i in range(SIDE_LENGTH)]

    # Find status of 2 diagonals
    main_diag = [row[i] for i, row in enumerate(grid)]
    counter_diag = [row[-i - 1] for i, row in enumerate(grid)]

    # Winning conditions for each player
    if any((any(row == row_os for row in grid),
            any(col == row_os for col in columns),
            any(diag == row_os for diag in [main_diag, counter_diag]))):
        print("The computer has won!")
        sys.exit()
    elif any((any(row == row_xs for row in grid),
            any(col == row_xs for col in columns),
            any(diag == row_xs for diag in [main_diag, counter_diag]))):
        print("Well done, you have won!")
        sys.exit()
    else:
        # If nobody has won yet, take another turn
        if turn % 2 == 0:
            x_coord = input()
            y_coord = input()
            coords_picked.add((int(x_coord), int(y_coord)))
            grid[int(x_coord)][int(y_coord)] = "X"
            print(format_grid(grid))
            turn += 1
        else:
            # Computer randomly picks a co-ordinate in the grid that has not been picked yet
            remaining_coords = all_coords - coords_picked
            comp_coords = random.choice(list(remaining_coords))
            grid[comp_coords[0]][comp_coords[1]] = "O"
            coords_picked.add((comp_coords[0], comp_coords[1]))
            print(comp_coords)
            print(format_grid(grid))
            turn += 1

print("A draw?! We must play again!")


Followup: How do I optimize this Java cube root function for BigInteger? Followup to How do I optimize this Java cube root function for BigInteger?
So I've tried several implementations of this algorithm. The version using only BigInteger sometimes results in a never-ending cycle of candidates:
public static final BigInteger _3=BigInteger.valueOf(3);

public static BigInteger cbrt(BigInteger num)
{
 BigInteger root=BigInteger.ZERO.setBit(num.bitLength()/3),temp;
 do {
  temp=root;
  root=temp.add(temp).add(num.divide(temp.multiply(temp))).divide(_3);
 } while(!root.equals(temp));
 return root;
}

This hangs the program for certain numbers (5 and 26 are prime examples).
Next I tried using BigDecimal to give me more accurate division, but I was afraid that converting back and forth between BigInteger and BigDecimal might be slowing it down:
public static final BigDecimal _3=BigDecimal.valueOf(3);
public static final int UP=BigDecimal.ROUND_HALF_UP;

public static BigInteger cbrt(BigInteger num)
{
 BigDecimal numD=new BigDecimal(num),temp;
 BigInteger root=BigInteger.ZERO.setBit(num.bitLength()/3);
 do {
  temp=new BigDecimal(root);
  root=temp.add(temp).add(numD.divide(temp.multiply(temp),UP)).divide(_3,UP).toBigInteger();
 } while(!root=temp.toBigInteger());
 return root;
}

On a suggestion from the comments, I tried modifying the first function above with the algorithm ending with the root less than or equal to the temp. But that returns the wrong root for some numbers:
public static final BigInteger _3=BigInteger.valueOf(3);

public static BigInteger cbrt(BigInteger num)
{
 BigInteger root=BigInteger.ZERO.setBit(num.bitLength()/3),temp;
 do {
  temp=root;
  root=temp.add(temp).add(num.divide(temp.multiply(temp))).divide(_3);
 } while(root.compareTo(temp)>0);
 return root;
}

Try 1367631. The return value is 113, not 111.
Then there's my solution: multiple temporary values:
public static final BigInteger _3=BigInteger.valueOf(3);

public static BigInteger cbrt(BigInteger n)
{
 BigInteger root=BigInteger.ZERO.setBit(n.bitLength()/3),t1,t2,t3;
 t1=t2=t3=BigInteger.ZERO;
 do {
  t3=t2;t2=t1;t1=root;
  root=t1.add(t1).add(n.divide(t1.multiply(t1))).divide(_3);
 } while(!root.equals(t1.add(t2).add(t3).divide(_3)));
 return root;
}

I'm trying to decide between the BigDecimal version and the multiple-temp version. Any suggestions how I can make either algorithm better?
Edit: Followup can be found here: https://codereview.stackexchange.com/questions/56862

I think your final solution is on the right path.

Just to be clear, I'm assuming you want the integral part of the cube root, i.e. for a given \$n\$, cbrt(n) will return the integer \$m\$ such that \$m^3 \leq n < (m + 1)^3\$. I'm also assuming you only care about positive inputs.

Here is the code I posted in a comment on the previous question, with an improved initial estimate. Inspired by There are Only Four Billion Floats -- So Test Them All!, I have tested it successfully on all integers in the range [1, Integer.MAX_VALUE) (took about 40m, by the way).

private static final BigInteger THREE = BigInteger.valueOf(3);

private static BigInteger cubeRoot(BigInteger n) {
    // Using Newton's method, we approximate the cube root
    // of n by the sequence:
    // x_{i + 1} = \frac{1}{3} \left( \frac{n}{x_i^2} + 2 x_i \right).
    // See http://en.wikipedia.org/wiki/Cube_root#Numerical_methods.
    //
    // Implementation based on Section 1.7.1 of
    // "A Course in Computational Algebraic Number Theory"
    // by Henri Cohen.
    BigInteger x = BigInteger.ZERO.setBit(n.bitLength() / 3 + 1);
    while (true) {
        BigInteger y = x.shiftLeft(1).add(n.divide(x.multiply(x))).divide(THREE);
        if (y.compareTo(x) >= 0) {
            break;
        }

        x = y;
    }

    return x;
}

The test code:

for (int i = 1; i < Integer.MAX_VALUE; i++) {
    // Report progress.
    if (i % 1000000 == 0) {
        System.out.printf("%d%n", i);
    }

    BigInteger n = BigInteger.valueOf(i);
    BigInteger m = cubeRoot(n);

    BigInteger lower = m.pow(3);
    BigInteger upper = m.add(BigInteger.ONE).pow(3);

    if (lower.compareTo(n) <= 0 && n.compareTo(upper) < 0) {
        continue;
    }

    System.err.printf("Error for input %s: Got %s%n", n, m);
    System.err.printf("Expected m^3 <= %s < (m + 1)^3%n", n);
    System.err.printf("But m^3 = %s, (m + 1)^3 = %s%n", lower, upper);
}


Python coroutine structure I've completed Advent of Code's Day 9 in Python3, trying to use it to teach myself coroutines. I'm very familiar with generators, but I've pretty much never used coroutines before.

I've ended up with something that works (I got the answers correct for both parts of the challenge), but my use of coroutines feels really fragile in some poorly-defined sense, and I was particularly disappointed that I couldn't make a program in which I used yield from; I thought that this was the kind of thing that yield from was built for.

Overall strategy: input an iterable string containing the expression to parse. Hand that iterable to various generators in turn, depending on what is required; those consume the characters they need and then yield their answers before being destroyed.

A group is as defined in the problem: anything contained in {} in the input. score is the "score" of the input (i.e. the number of groups in the tree or in any subtree). garbage_len is the total length of the garbage objects in the tree or any subtree. I'm not really concerned with how to improve those two methods; I'd like feedback on the idiomatic use of generators/coroutines.

class color:
    RED = '\033[91m'
    END = '\033[0m'

class Garbage:
    def __init__(self):
        self.characters = []

    def __str__(self):
        return '{}{}{}'.format(color.RED, ''.join(str(s) for s in self.characters), color.END)

    def add(self, ch):
        self.characters.append(ch)

    def __len__(self):
        return len(self.characters)

    def __bool__(self):
        return True

class States(enum.Enum):
    Group = 0
    Garbage = 1
    Escaped = 2

class Group:
    def __init__(self):
        self.contents = []

    def __str__(self):
        # A group contains a list of {Garbage or Group}.
        return '{' + ','.join(str(s) for s in self.contents) + '}'

    def score(self, depth=0):
        ans = depth + 1
        for g in self.contents:
            if isinstance(g, Group):
                ans += g.score(depth + 1)
        return ans

    def garbage_len(self):
        ans = 0
        for gr in self.contents:
            if isinstance(gr, Group):
                ans += gr.garbage_len()
            else:
                assert isinstance(gr, Garbage)
                ans += len(gr)
        return ans

def curr_garbage(gen):
    # Coroutine which consumes garbage characters and yields characters.
    # Stops when the current garbage is finished.
    state = States.Garbage
    for ch in gen:
        if state == States.Escaped:
            state = States.Garbage
        elif state == States.Garbage:
            if ch == '!':
                state = States.Escaped
            elif ch == '>':
                return
            else:
                yield ch

def curr_group(gen):
    # Coroutine which consumes characters and yields Groups and Garbages.
    for ch in gen:
        if ch == '{':
            # New group.
            subgroup = Group()
            subgroup_gen = curr_group(gen)
            subgroup.contents = list(sub)
            yield subgroup
        elif ch == ',':
            pass
        elif ch == '}':
            # End of current group
            raise StopIteration
        elif ch == '<':
            # New garbage
            garbage = Garbage()
            garbage_gen = curr_garbage(gen)
            for garbage_char in garbage_gen:
                garbage.add(garbage_char)
            yield garbage
        else:
            raise ValueError("Malformed string, received character {}".format(ch))

if __name__ == '__main__':
    gen = curr_group(iter(line))
    tree = next(gen)
    print(tree.score())
    print(tree.garbage_len())

The expression ''.join(str(s) for s in self.characters simply returns a copy of self.characters. Consider phrasing it

    return color.RED + self.characters + color.END

Similarly ','.join(str(s) for s in self.contents seems longer than necessary.

This does not appear to be used:

    def __bool__(self):
        return True

I'm afraid you lost me here:

    gen = curr_group(iter(line))

I don't see any mentions that define line.

Yielding a value to co-routine seems to be working fine, just as you wish.


command-line binary/hex conversion program in python I'm just learning the ropes to programming, so bear with me here. I wanted a simple command-line program to quickly convert decimal numbers into either binary or hexadecimal. This is what I came up with, but I know it can be improved.
while True:
    while True:
        conv_type = input("Hexadecimal or Binary?: ").lower()
        if "b" not in conv_type and "h" not in conv_type:
            print("Invalid Input")
            continue
        else:
            break

    while True:
        try:
            dec_num = int(input("Decimal Number: "))
        except ValueError:
            print("Invalid Input")
            continue
        break

    if "h" in conv_type:
        print("   ", dec_num, "=", hex(dec_num), "in hexadecimal")

    if "b" in conv_type:
        print("   ", dec_num, "=", bin(dec_num).replace("0b", ""), "in binary")

How would you improve this? I feel a bit weird using three while True loops in the same program, but I couldn't think of a better solution on the fly. Any feedback is greatly appreciated!

User input validation is annoying, right? My instinct is to ask fewer
questions. Does the user really need to ask for hex or binary? For example, we
could just print both conversions. And if it's a command-line script, couldn't
the decimal number come directly from the command line? Or even a bunch of
decimal numbers and our script could convert them all.
But if we retain your current approach, the repetitiveness of the validation
suggests the need to move code into functions and to create a single function
to handle all input() calls. At a minimum, that function needs a user-prompt
and a function (or callable) that will convert the value and raise an error if
it's invalid. For example, to validate conversion type, we need to lowercase
the input and then make sure it contains a b or h. While we're at it, we
could simplify downstream logic by defining constants for conv_type.
Subsequent code can use strict equality to drive logic (rather than
looser in checks). And in this case, the constant can also serve as a label
when printing.
A consolidated approach to getting user input allows you to address
another issue: how does the user quit the program? However you
decide to handle that, it's handy to be able to do it in one place
rather than multiple.
If you strip off the 0b prefix for binary shouldn't you do the same for hex?
If so, that simplifies the logic, because we can extract the things that vary
into a data structure (converters) and then use that structure to drive the
logic. The result is that most of the "algorithm" in the main() function
fades away and we end up with very little conditional branching. Instead, it
just marches step by step, calling utility functions or accessing data
structures directly. Instead, the algorithmic complexity is delegated to
smaller and more narrowly focused utility functions that are easier to think
about in isolation.
import sys

HEX = 'hexadecimal'
BIN = 'binary'

def main(args):
    # Set up conversion functions and prefixes.
    converters = {
        HEX: (hex, '0x'),
        BIN: (bin, '0b'),
    }

    # Perform conversions.
    while True:
        # Get validated input. Since int() will raise if given
        # a bad input, we get that validation for free.
        conv_type = get_user_input('Hexadecimal or binary', is_conv_type)
        dec_num = get_user_input('Decimal number', int)

        # Convert.
        func, prefix = converters[conv_type]
        converted = func(dec_num).replace(prefix, '')

        # Report.
        print(f'    {converted} = in {conv_type}')

def get_user_input(prompt, validate):
    while True:
        # Get input and provide a way for user to quit easily.
        reply = input(prompt + ': ').lower().strip()
        if not reply:
            sys.exit()

        # Return valid converted input or nag the user.
        try:
            return validate(reply)
        except ValueError as e:
            print(f'Invalid input: {reply}')

def is_conv_type(reply):
    # Takes user input. Returns HEX or BIN if valid.
    reply = reply.lower()
    for conv_type in (HEX, BIN):
        if conv_type[0] in reply:
            return conv_type
    raise ValueError()

if __name__ == '__main__':
    main(sys.argv[1:])


Python Palindrome Generator I've just written a palindrome generator and would like to know what you think about it.

Especially:

code style
Is it possible to get the same result without making a special case for 1-digit palindromes?
efficiency

#!/usr/bin/env python

def getPalindrome():
    """
        Generator for palindromes.
        Generates palindromes, starting with 0.
        A palindrome is a number which reads the same in both directions.
    """
    # First print all one-digit palindromes
    for i in range(10):
        yield i

    length = 2
    while True:
        # Do the symmetric part
        for i in range(10**(length//2-1), 10**(length//2)):
            a = str(i)
            r = a + a[::-1]
            yield int(r)
        length += 1

        # Do the unsymmetric part
        exponent = (length-1)//2
        for prefix in range(10**(exponent-1), 10**exponent):
            prefix = str(prefix)
            for i in range(10):
                result = prefix + str(i) + prefix[::-1]
                yield int(result)
        length += 1

if __name__ == "__main__":
    palindromGenerator = getPalindrome()
    for i, palindromeNumber in enumerate(palindromGenerator):
        print("%i-th palindrome: %i" % (i, palindromeNumber))
        if i >= 500:
            break

I'd implement this function like this:

from itertools import count

def palindromes():
    """Generate numbers that are palindromes in base 10."""
    yield 0
    for digits in count(1):
        first = 10 ** ((digits - 1) // 2)
        for s in map(str, range(first, 10 * first)):
            yield int(s + s[-(digits % 2)-1::-1])

(In Python 2, you should replace range with xrange and map with itertools.imap.)

The insight is that instead of building an odd-length palindrome from the parts prefix + middle + xiferp, we can build it as prefix + iferp: that is, with the middle digit being supplied by the last digit of the prefix.
In your main loop, you arrange to generate the first 501 palindromes:

for i, palindromeNumber in enumerate(palindromGenerator):
    ...
    if i >= 500:
        break

but it would be simpler to use itertools.islice:

for i, palindrome in islice(enumerate(palindromeGenerator), 501):
    ...

As for your question about efficiency, it is impossible to answer without knowing the purpose of this code. If all you are going to do is to print the first 501 palindromes, then your code is fine: the runtime will be dominated by printing the output.


Modeling and computing intersection between continuous intervals I created the class ContinuousDimensionInterval, to model an interval of a continuous dimension interval, presented below.

namespace Minotaur.Math.Dimensions {
    using System;

    public sealed class ContinuousDimensionInterval: IDimensionInterval {

        public int DimensionIndex { get; }
        public readonly DimensionBound Start;
        public readonly DimensionBound End;

        public ContinuousDimensionInterval(
            int dimensionIndex,
            DimensionBound start,
            DimensionBound end
            ) {
            if (dimensionIndex < 0)
                throw new ArgumentOutOfRangeException(nameof(dimensionIndex) + " must be >= 0.");

            DimensionIndex = dimensionIndex;
            Start = start;
            End = end;
        }

        public bool Contains(float value) {
            if (float.IsNaN(value))
                throw new ArgumentOutOfRangeException(nameof(value));

            if (value > Start.Value && value < End.Value)
                return true;
            if (Start.IsInclusive && value == Start.Value)
                return true;
            if (End.IsInclusive && value == End.Value)
                return true;

            return false;
        }
    }
}

I felt the need to create a struct, DimensionBound, to represent the bounds of an interval, since they may be inclusive or exclusive.

namespace Minotaur.Math.Dimensions {
    using System;

    public readonly struct DimensionBound {
        public readonly float Value;
        public readonly bool IsInclusive;

        public DimensionBound(float value, bool isInclusive) {
            if (float.IsNaN(value))
                throw new ArgumentOutOfRangeException(nameof(value) + " can't be NaN.");

            Value = value;
            IsInclusive = isInclusive;
        }
    }
}

I'd like to receive some feedback on the implementation of both classes.

Should I disallow the construction of DimensionBound with
value=infinity and inclusive=true?
Is my decision to make DimensionBound a struct reasonable? It feels struct-like to me...
Should DimensionBound implement IEquatable<DimensionBound>?
Am I over-engineering the whole thing by creating a class to represent the bounds of an interval?

Here an example of how the class is used:

private static bool IntersectsContinuous(ContinuousDimensionInterval lhs, ContinuousDimensionInterval rhs) {
    // @Improve performance?
    // @Verify correctness
    var aStart= lhs.Start;
    var aEnd = lhs.End;

    var bStart = rhs.Start;
    var bEnd = rhs.End;

    if (aStart.Value > bEnd.Value)
        return false;
    if (aStart.Value == bEnd.Value)
        return aStart.IsInclusive && bEnd.IsInclusive;

    if (bStart.Value > aEnd.Value)
        return false;
    if (bStart.Value == aEnd.Value)
        return bStart.IsInclusive && aEnd.IsInclusive;

    return true;
}

Review

You are constrained to float which makes your class only usable for a limited number of scenarios. I would like to be able to work with int, double, DateTime, MyCustomEntity, ..
You should not need to care about special cases as unbounded or NaN. NaN should be filtered out beforehand, since it is unusable for comparison. Unbounded and bounded values should be compared by using the built-in interface IComparable.
Intersection of continuous data when 2 edges match requires both edges to be IsInclusive in your specification. I would argue that only one of them should require this flag. Adjacent items that meet each other form an intersection ( [source required] ).
Your struct is immutable, which is proper design. Whether or not to make a class instead, depends on how you would like to use this. I tend to prefer class more than struct. Note that a struct has an inherent default value, while a reference type is null by default. How would you distinguish a default value from a value that happens to have the same value? This is one reason to go with a class, unless you cope with this issue by using a Nullable<T> and accepting the overhead it provides.
It is good practice to override GetHashCode() and Equals().

Suggestions

1201ProgramAlarm's answer suggests to use IComparable on DimensionBound, but I would use it on the generic type parameter.

public class DimensionBound<T> where T : IComparable<T>
{
    public readonly T Value;
    public readonly bool IsInclusive;

    public DimensionBound(T value, bool isInclusive)
    {
        Value = value;
        IsInclusive = isInclusive;
    }
}

The interval could then be refactored to compare the values. Note that I would provide a convenience constructor accepting an inclusive start and exclusive end (which is considered common practice when dealing with continuous data).

I would also favour instance methods over the static comparison. Furthermore, I would argue the purpose of the dimension in this class. I would remove it and call the class Interval. You can always create an additional class or use a tuple (int, Interval) to include the dimension.

public sealed class Interval<T> where T : IComparable<T>
{
    public readonly DimensionBound<T> Start;
    public readonly DimensionBound<T> End;

    public Interval(T start, T end)
        : this(ew DimensionBound<T>(start, true), new DimensionBound<T>(end, false))
    {
    }

    public Interval(DimensionBound<T> start, DimensionBound<T> end)
    {
        Start = start;
        End = end;
    }

    public bool Contains(T value)
    {
        // method body ..
    }

    public bool Intersects(Interval<T> other)
    {
        // method body ..
    }
}

If we don't change your specification and require both edges to be inclusive when 2 intervals meet, we could implement Intersects (Contains is similar) as follows (comments added for code review purposes only):

public bool Intersects(Interval<T> other)
{
    // compare start to other end
    var compareToStart = Start.Value.CompareTo(other.End.Value);

    // no intersection possible; instance starts after other is finished
    if (compareToStart == 1) return false;

    // both intervals meet but at least one of the bounds is exclusive, so no intersection
    if (compareToStart == 0 && !(Start.IsInclusive && other.End.IsInclusive)) return false;

    // compare end to other start
    var compareToEnd = End.Value.CompareTo(other.Start.Value);

    // no intersection possible; instance finishes before other is started
    if (compareToEnd == -1) return false;

    // both intervals meet but at least one of the bounds is exclusive, so no intersection
    if (compareToEnd == 0 && !(End.IsInclusive && other.Start.IsInclusive)) return false;

    return true;
}

This works with float, or any other IComparable<T>.

Assert.IsTrue(new Interval<float>(0, 1).Intersects(new Interval<float>(0.5f, 1.5f));