Verification of Sudoku boards A valid Sudoku board has the following properties

The board must be square (n-by-n). Let m = sqrt(n)
All rows must contain the numbers 1-n
All columns must contain the numbers 1-n
All nine m-by-m blocks must contain all numbers 1-n

The following MATLAB code verifies a Sudoku board of arbitrary sizes (where n is a square number):

function correct_board = sudoku_checker(board)

correct_board = true;
[rows, cols] = size(board);
correct_size = (rows == cols) && mod(sqrt(rows),1) == 0;
if correct_size == false
    disp('The board is not square')
    correct_board = false
else
    disp('The board is square')
    for ii = 1:rows
        if ~(isequal(unique(board(:,ii)), (1:rows)') && isequal(unique(board(ii,:)), (1:rows)))
            disp(['Numbers ' num2str(1) '-' num2str(rows) ' are not present in all rows or columns'])
            correct_board = false;
            return
        end
    end
    disp(['Numbers ' num2str(1), '-' num2str(rows) ' are present in all rows and columns'])

    cell_blocks = mat2cell(board, repmat(sqrt(rows),sqrt(rows),1),repmat(sqrt(rows),sqrt(rows),1));
    blocks_ok = all(arrayfun(@(x) isequal(unique(cell_blocks{x}),(1:rows)'), 1:rows));
    if blocks_ok == true
        disp(['All blocks are OK'])
    else
        disp(['All blocks are not OK'])
        correct_board = false;
        return;
    end
    disp('Board is OK')
end
end

A valid board will give the following output:

sudoku_checker(board)
The board is square
Numbers 1-9 are present in all rows and columns
All blocks are OK
Board is OK

An incorrect board will give an output like this (dependent on which condition that fails):

sudoku_checker(board)
The board is square
Numbers 1-4 are present in all rows and columns
All blocks are not OK

I'm wondering if there are ways to improve this code. I'm looking for improvements in performance and algorithm, as well as improvement in coding style etc.

Can I simplify any expressions? Remove some boolean operators?

A few general things first:

' is the conjugate transpose, not the transpose. These are the same for real values, but will behave differently when you have complex values. To avoid confusion, it is recommended that you always use .' when transposing a matrix, even if it's real.

You're using both disp('text') and disp('[text]'). The first one works for plain strings, while the other must be used when concatenating strings and numbers. I suggest you either use brackets on all calls to disp, or skip the brackets for all calls to disp where the string is plain text. Consistency is always nice.

Variable names:

The variable names are well chosen and descriptive. One minor comment is that you're using correct_size and correct_board, but blocks_ok. correct_blocks is probably a better word here, to keep consistency.

Frequently used variables:

There are a few variables that are used over and over again. You should declare the following variables outside the loop:

sq_rows = sqrt(rows);
values= 1:rows;

Avoid the loop and calls to unique by sorting the board by rows and columns:

You can avoid the loops by sorting all columns of the regular board, and the board when it's transposed, instead of checking for unique values.

isequal(sort(board), sort(board.'), repmat(values.',1, rows))

repmat(values.',1,rows) produces a matrix looking like this:

1     1     1     1
2     2     2     2
3     3     3     3
4     4     4     4

Alternatives to repmat(sqrt(rows),sqrt(rows),1):

Using mat2cell to create cell_blocks is good, but the expression can be simplifed using the new pre-defined variables. There are some alternatives to repmat(sqrt(rows),sqrt(rows),1) that are more readable, such as:

ones(sq_rows,1) * sq_rows;
zeros(sq_rows,1) + sq_rows;

repelem is an option if you're using MATLAB version R2015a or newer.

Use cellfun when you're working with cells, and arrayfun when you are working with other "stuff".

Since cell_blocks is a cell array, you can use cellfun:

all(cellfun(@(x) isequal(unique(x),values.'), cell_blocks))

Simplifying the disp-calls:

You know that the first number in the sequence is 1, so you can simplify the two disp-calls in the middle:

disp(['Numbers 1-' num2str(rows) ' are present in all rows and columns'])

I believe you're missing a break after checking the size of the board (first if).

Comments!

I suggest you use a lot more comments! Explain what the different parts of your code do.


List all possible permutations from a python dictionary of lists The following code takes a dictionary of lists (indexed by keyword)  and converts it into a list of list of dictionaries that contains all possible permutations of those lists.  However, my solution seems hackish, and is certainly space inefficient.. I feel like I'm missing something obvious.

The use case is for enumerating all the possible hyperparameters for a neural network (think sklearn grid search).  So I specify the options in a compact dictionary containing the name of the parameter, and a list of values to test, and then convert that into a list of single-entry-per-key dictionaries that contain all possible permutations of those configuration parameters.

It's important to note I'm trying to make this generic, so that there can be an arbitrary number of keys specified in the initial dictionary, with an arbitrary number of options for each key.

Here's the code:

import copy

config_overrides = {
    'nodetype': ['srnn', 'lstm', 'gru'],
    'nhidden': [1,2,3,4,5,6,8,10,12,16,20],
    'opt': ['sgd', 'rmsprop', 'adagrad', 'adamax', 'adam', 'nadam', 'adadelta'],
    'actfunc': ['softplus', 'softsign', 'tanh', 'sigmoid', 'hard_sigmoid', 'linear', 'relu'],
    'loss': ['mse', 'msle']
}

experiments = [{},]

for k,v in config_overrides.items():
    new_values = len(v)
    current_exp_len = len(experiments)
    for _ in range(new_values-1):
       experiments.extend(copy.deepcopy(experiments[:current_exp_len]))
    for validx in range(len(v)):
       for exp in experiments[validx*current_exp_len:(validx+1)*current_exp_len]:
            exp[k] = v[validx]

print(len(experiments))
print([x for x in experiments[1034:1039]])

The output of the above program, which includes random outputs from the middle of the list, and shows it appears tobe working correctly (the above example should come out to 3234 permutations):

3234
[{'loss': 'mse', 'opt': 'adadelta', 'actfunc': 'sigmoid', 'nodetype': 'lstm', 'nhidden': 4}, {'loss': 'msle', 'opt': 'adadelta', 'actfunc': 'sigmoid', 'nodetype': 'lstm', 'nhidden': 4}, {'loss': 'mse', 'opt': 'sgd', 'actfunc': 'hard_sigmoid', 'nodetype': 'lstm', 'nhidden': 4}, {'loss': 'msle', 'opt': 'sgd', 'actfunc': 'hard_sigmoid', 'nodetype': 'lstm', 'nhidden': 4}, {'loss': 'mse', 'opt': 'rmsprop', 'actfunc': 'hard_sigmoid', 'nodetype': 'lstm', 'nhidden': 4}]

A few immediate questions:

Is list.extend() the best way to expand the list?
The deepcopy() is necessary, or else you end up with copies of the original dictionary. But that also means the strings are copied, redundantly.  Not a huge deal, but doesn't feel right to me.

Other than that, it just seems like there should be a cleaner, less obtuse way of doing this..and maybe I'm just too tired to think of it.  Any ideas?

The tool you need here is itertools.product:

>>> import itertools
>>> keys, values = zip(*config_overrides.items())
>>> experiments = [dict(zip(keys, v)) for v in itertools.product(*values)]
>>> len(experiments)
3234
>>> experiments[1034]
{'nhidden': 4, 'nodetype': 'lstm', 'loss': 'msle', 'opt': 'nadam', 'actfunc': 'sigmoid'}

Of course, if you are going to be iterating over the experiments one by one, then there is no point in allocating a list, and you could do this instead:

keys, values = zip(*config_overrides.items())
for v in itertools.product(*values):
    experiment = dict(zip(keys, v))
    # etc.

And depending on what you are doing with the experiments, you may not even need to build the dictionary:

keys, values = zip(*config_overrides.items())
for experiment in itertools.product(*values):
    for key, value in zip(keys, experiment):
        # etc.


Slim down Javascript MomentJS Function I have a drop down menu that will run a function onchange. The function takes times from an array , adds a specific amount of time , then changes the innerHTML of the table.

The code works , but is there a simpler version? I plan on adding more options to the drop down menu.

HTML

<select id="sel1" onchange="myFunction()">

   <option value="VIC">VIC</option>
   <option value="NSW">NSW</option>

</select>

        A
        2016-08-24 16:00
        C

        A
        2016-08-24 16:00
        C

Javascript

function myFunction(){

var cellData = ["2016-08-24 15:45", "2016-08-24 16:00"];
var myTable = document.getElementById("myTable");

if (document.getElementById("sel1").value == "VIC") {

var vicTime = [];

for (i=0; i<cellData.length; i++) {

date = moment(cellData[i]).add(1, 'h').format('MM/DD/YYYY hh:mm a');

vicTime.push(date);

myTable.rows[i].cells[1].innerHTML = vicTime[i];

}}

if (document.getElementById("sel1").value == "NSW") {

var nswTime = [];
var j;

for (j=0; j<cellData.length; j++) {

date = moment(cellData[j]).add(5, 'h').format('MM/DD/YYYY hh:mm a');

nswTime.push(date);

myTable.rows[j].cells[1].innerHTML = nswTime[j];

}}};

When you find yourself repeating code like this (just with slight differences), that's usually a sign that it's time to separate the logic into its own function so it can be reused. You could also maybe clean up your conditionals using a switch statement too - but that's more of a personal taste thing.

Here's my take on it:

function myFunction() {
    var cellData = ["2016-08-24 15:45", "2016-08-24 16:00"];
    var myTable = document.getElementById("myTable");

    // Defined inside the function so the data/table are in scope -
    // alternatively, you could define it outside and pass them as
    // parameters.
    function setCells(hours) {
        var times = [];

        for (var i = 0; i < cellData.length; i++) {
            var date = moment(cellData[i]).add(hours, 'h').format('MM/DD/YYYY hh:mm a');
            times.push(date);

            myTable.rows[i].cells[1].innerHTML = times[i];
        }
    }

    switch (document.getElementById("sel1").value) {
        case "VIC":
            setCells(1);
            break;
        case "NSW":
            setCells(5);
            break;
    }
}

And a demo of this code in action: https://jsfiddle.net/5q46xu77/1/


Three ways to add pronoun method/property to gendered class in Python Say you have a Python class for something that has an optional gender field. You'd like a helper property/method to get the appropriate pronoun for an instance. You'd also like to be able to get the possessive version of the pronoun. The use for these helpers will be in creating human-readable emails and other such messages related to the object.

Below are three ways to do it. Which is best in terms of style and design and why?

Option 1

@property
def pronoun(self):
    return "he" if self.gender == 'male' else "she" if self.gender == 'female' else "they"

@property
def possessive_pronoun(self):
    return "his" if self.gender == 'male' else "her" if self.gender == 'female' else "their"

Option 2

def pronoun(self, possessive=False):
    if possessive:
        return "his" if self.gender == 'male' else "her" if self.gender == 'female' else "their"
    else:
        return "he" if self.gender == 'male' else "she" if self.gender == 'female' else "they"

Option 3

def pronoun(self, possessive=False):
    pronouns = ("he", "his") if self.gender == 'male' else ("she", "her") if self.gender == 'female' \
        else ("they", "their")
    return pronouns[1] if possessive else pronouns[0]

Feel free to suggest an even better fourth option.

P.S. In case you are wondering, I like to use double quotes for strings meant for human consumption, single quotes otherwise.

I usually try to use data structure over code whenever it's possible.
It usually make things shorter, easier to read and easier to update.

If I had to write such a thing, I'd probably write something like (untested code but just to show the idea) :

pronouns = {
    'male' => ("he","his"),
    'female' => ("she", "her")
}

def pronoun(self, possessive=False):
    return pronouns.get(self.gender, default=("they","their"))[possessive]

If we could be sure that self.gender has only 3 different values, the default value could be put in the pronouns dictionnary straightaway (associated to "plural" for instance).

Different variations could be written. I guess one could use defaultdict to ensure that we always retrieve the default value. I don't like this solution that much as the dictionnary would get bigger as we try to retrieve using invalid keys.

I don't know if there's a (simple) way to have a dictionnary which returns a default value when the key is not present without updating the dictionnary but I'd be interested in such a thing.


Reversing the "words" of a container with a bidirectional iterator I posted another snippet of code a few weeks back when I started learning C++. Here's my second try now that I have some time to play with this again. The goal is to write a generic algorithm for reversing the "words" of a container with a bidirectional iterator. Separators of words are any values that a functor parameter deems as a separator.

#include <iostream>
#include <utility>
#include <string>
#include <cctype>

template<typename BidirIt>
void my_reverse(BidirIt first, BidirIt last)
{
  for(--last; first < last; ++first, --last)
    std::swap(*first, *last);
}

template<typename BidirIt, typename IsSeparatorTester>
void reverse_words(BidirIt first, BidirIt last, IsSeparatorTester issep)
{
  my_reverse(first, last);

  for(auto it = first; it != last; ++it) {
    if(issep(*it)) {
      my_reverse(first, it);
      first = it;
      ++first;
    }
  }

  // we bail out of the loop before inverting last word
  my_reverse(first, last);
}

int main()
{
  std::string test { " this   is a   test   " };
  std::cout << test << std::endl;
  reverse_words(test.begin(), test.end(), isspace);
  std::cout << test << std::endl;
  return 0;
}

As a comment, I know that STL has a reverse procedure, but I wanted to try to implement something similar myself.

Although not relevant to the algorithm itself, you could still consider these basic things:

It's best to use "\n" instead of std::endl for newlines.
You don't need a return 0 at the end of main(); it will do this for you automatically.
You should also add curly braces in my_reverse() for consistency and in case you'll need to add more to the loop.
The name IsSeparatorTester issep seems odd as an argument. Perhaps it should be renamed to something like Separator sep since it's not performing an action (like a function) and should therefore be a noun.

However, if you're mostly trying to reimplement std::reverse(), then you can leave it out this argument altogether.  There may not be many cases in which the user would choose something other than a space as a separator.


HackerRank "Manasa and Stones" in Python Problem Statement:

Manasa is out on a hike with friends. She finds a trail of stones with numbers on them. She starts following the trail and notices that two consecutive stones have a difference of either a or b. Legend has it that there is a treasure trove at the end of the trail and if Manasa can guess the value of the last stone, the treasure would be hers. Given that the number on the first stone was 0, find all the possible values for the number on the last stone.
Note : The numbers on the stones are in increasing order

Input Format:
The first line contains an integer T i.e. the number of Test cases. T testcases follow.
Each testcase has 3 lines. The first line contains n ( the number of stones ) The second line contains a. The third line contains b.
Output Format:
Space separated list of numbers which are the possible values of the last stone in increasing order.
Constraints:
1 ≤ T ≤ 10
1 ≤ n, a, b ≤ 103
Algorithm:
I order a and b such that a > b. The maximal value of the last stone is then found from the sequence:
1, a, 2 * a, 3 * a, ..., (n-2) * a, (n-1) * a

Which is arithmetic sequence with difference a. A smaller value will be produced by replacing the last difference of 'a' with 'b', which results in (n-2) * a + 1 * b. Doing this repeatedly yields the list of possible stone values:
(n-1) * a + 0 * b, (n-2) * a + 1 * b, ..., 1 * a + (n-2) * b, 0 * a + (n-1) * b

Since a, b can be equal, I wrap this list into a set to produce my code below.
Code:
T = int(input())

for _ in range(T):
    n = int(input())
    a = int(input())
    b = int(input())
    a, b = sorted((a, b))
    values = sorted(set([(n - i - 1) * a + i * b for i in range(n)]))
    print(*values)

I would like any suggestions to help improve this code.

Since a, b can be equal, I wrap this list into a set to produce my
  code below.

Why not just handle this case separately? It's then simple to prove that if a != b, we don't have repeated values, and we can just generate them in increasing order.

def last_stone(a, b, n):
    if a == b:
        return [a * (n - 1)]
    if a < b:
        return last_stone(b, a, n)
    return [i * a + (n - i - 1) * b for i in range(n)]


Speed up performance of nested for loops to get all related format of all Authors' Books This works but is very slow.  I have an Author table that foreign keys to all books by that author.  Each book has a variety of formats it can appear in (html, pdf, etc), which its linked to by its own foreign key.

As the code shows, what I'm doing now is getting all the authors, then looping through to get each book by the author, and looping through those to get all the format types of each book.  If the book does not have all four types, it's appended to a list of dictionaries to be displayed in the template.

Note that checking simply the count of each format would be insufficient, as it's possible a book might have multiple versions of one type; I need to make sure each type appears at least once.

We have tens of thousands of authors, each of which can have up to about 20 books, each of which can have up to about 5-8 formats max.

def missing_book_format_report(request):

    authors = Author.objects.all()
    data = []
    for author in authors:
        books = author.book_set.all()
        for book in books:
            book_info = {
                "book_num": book.num,
                "type": book.short_name,
                "xml": False,
                "pdf": False,
                "html": False,
                "gen_html": False
            }
            book_formats = book.format_set.all()
            for book_format in book_formats:
                format_type = book_format.format_type_cd_id.lower()  # e.g. xml
                book_info[format_type] = True
            if not all([book_info["xml"], book_info["pdf"], book_info["html"], book_info["gen_html"]]):
                data.append(book_info)
    context = {
        'data': data,
        'books': books
    }

    return render(request, 'book-report.html', context)

If you would use a debugger or something like a django-debug-toolbar, you'd see that you are actually issuing a lot of queries - to get all authors, for each author to get all the books, for each book to get all the formats.

There is a specific tool in Django to solve this kind of problem of getting the related sets - prefetch_related() and select_related(). Replace:

authors = Author.objects.all()  # 1 query
for author in authors:
    books = author.book_set.all() # 1 query per each author

with:

# only 1 query
authors = Author.objects.prefetch_related('books').all()
for author in authors:
    books = author.books.all()

See also this nice article with related examples:

Solving Performance Problems in the Django ORM

For the multiple "prefetch" levels, please see:

prefetch_related for multiple Levels

You can also select only the columns you actually need - use values_list.


Type Name or Alias via Reflection for TT Code Generation I was working on some TT code generation to get rid of mundane C# codings in VS2013 Express Edition, and after researching online, created the following helper methods to facilitate Type declarations.

I am able to use it successfully in my code generation but wonder if they can be improved further or if there's any scenario which they do not cater for.

public static String GetActualTypeName(Type type, bool bAlias = true, bool bFullQualified = false)
{
    Type nullable = Nullable.GetUnderlyingType(type);
    String name = null;

    if (nullable != null) // handles Nullable types
    {
        name = (bAlias) ? GetAliasName(nullable, bFullQualified)
            : (bFullQualified) ? nullable.FullName : nullable.Name;

        return String.Format("{0}?", name);
    }

    if (!type.GenericTypeArguments.Any()) // handles non generic types
    {
        name = (bAlias) ? GetAliasName(type, bFullQualified)
            : (bFullQualified) ? type.FullName : type.Name;

        return name;
    }

    name = (bFullQualified) ? type.FullName : type.Name;

    // recrusive looping into generic type tree for the required type names
    String[] names = type.GenericTypeArguments
        .Select(t => GetActualTypeName(t, bAlias, bFullQualified))
        .ToArray();

    return String.Format("{0}<{1}>", name.Substring(0, name.IndexOf('`')), String.Join(",", names));
}

public static String GetAliasName(Type type, bool bFullQualified = false)
{
    String alias = null;

    using (var provider = new Microsoft.CSharp.CSharpCodeProvider())
    {
        var typeRef = new System.CodeDom.CodeTypeReference(type);

        alias = provider.GetTypeOutput(typeRef);
    }

    int index = alias.LastIndexOf(".");

    if (bFullQualified || index == -1)
        return alias;

    return alias.Substring(index + 1);
}

bool bAlias

Don't use Hungarian notation on names. When you look at the parameter, you can already see that it's a bool, there is no reason to repeat that in the name.

String name = null;

Since it seems you never want to use that null value, don't initialize the variable at all.

Also, it's more common to write string instead of String.

name = (bAlias) ? GetAliasName(nullable, bFullQualified)
    : (bFullQualified) ? nullable.FullName : nullable.Name;

What's the point of the parentheses around bAlias and bFullQualified. They don't seem to serve any purpose.

String[] names = type.GenericTypeArguments
    .Select(t => GetActualTypeName(t, bAlias, bFullQualified))
    .ToArray();

I think names is too general here. I think something like typeArgumentNames would be better here (even if it's significantly longer).

Also, String.Join() can accept IEnumerable<string>, so the ToArray() is unnecessary here.

name.Substring(0, name.IndexOf('`'))

Wouldn't this be more readable as name.Split('`')[0]? (It would also be less efficient, but that might not matter to you.)

String.Join(",", names)

I realize this is for generated code, so beauty does not matter much, but I would still prefer to add a space between the type arguments, i.e. String.Join(", ", names).

using (var provider = new Microsoft.CSharp.CSharpCodeProvider())

Neither CSharpCodeProvider not its base class, CodeDomProvider override Dispose(), it's only there because they inherit from Component.

This means that there is no need to use using here. But even if you know this, you might still choose to include the using, just to be on the safe side.

Also, since you're already using CSharpCodeProvider for this, why not use it to replace your whole GetActualTypeName()? You would lose the choice of specifying the form of the output, but it might be worth it for so much simplification.


Scheme, first timer first program, simple list removal technique I have started my hand at writing some scheme code. I am trying to get the first n primes. I plan to do so but first getting a list from 2 to m, and doing the following algorithm. Remove the first number and place in the primes list, remove all #'s from the initial list that are multiples of the number just pulled off, repeat until the size of the primes list equals n.

There are no loops in scheme so I have to use recursion and also modulo will probably help me, how might I go about writing the method "listminusnonprimes"? Also does my code so far look good?

Code:

;Builds a list from [arg] to 2
(define buildlist
  (lambda (m)
    (if (<= m 2)
      '(2)
        (cons m (buildlist(- m 1))))))
;Returns a list from 2 to [arg]
(define listupto
  (lambda (m)
    (reverse (buildlist m))))
;Returns a list with nonprimes based off num removed
(define listminusnonprimes
 (lambda (num list)
   (
;Returns the first n primes
(define firstnprimes
  (lambda (n nlist slist)
    (append (car nlist) slist)
    (if (= n (length slist))
        slist
        (firstnprimes (- n 1) (listMinusNonprimes (car nList) nlist) slit))))

In your function firstnprimes there is a typo near the end; more importantly it uses append but it is the wrong function to use there. What does append append? If called with two arguments the 2nd of which is a list, what must the first argument be - a list, or a value?

Another problem is your call (append (car nlist) slist) returns new value, a bigger list, but you do nothing with it. slist's value remains unchanged.

What are the two arguments of firstnprimes function, nlist and slist? What are their initial values? Will any two lists do? Obviously not. So, it seems better to define the real top-level function that is safe and easy to use:

(define myfirstnprimes
  (lambda (n)
    (firstnprimes n (listupto ...) (...))))

Something is missing there, isn't it? Some new, hidden parameter?

Now, you ask how to implement

(define listminusnonprimes
  (lambda (num list)
    ....

you call it as (listMinusNonprimes (car nList) nlist) so we know that initially, num is the first element of list; we also know that list is an ordered list of numbers increasing in value. So we just need to compare num with the list's first element:

   (let ((a (car list)))
     (cond
       ( (< num a) ... (listminusnonprimes ...) )
       ( ... )
       ( ... ))) ))

what are the cases? less-then, equal, greater-then, right? All that's left is to fill in the blanks. In particular, if the two are equal, both need to be removed and num changed to the next multiple of the prime; if num is greater, the top element should be kept; otherwise num should be changed to the next multiple of the prime. New missing parameters come into light here (prime... what prime? next multiple... how to find it?); also it isn't clear what does it mean "to keep" and "to remove", right?

About multiples, just write them down in a sequence first: p, 2p, 3p, 4p, .... Now devise a method of finding the next from the previous one. What information will you have to maintain?

About keeping/removing. There are two ways. One will lead to using set-cdr!, another to using cons, and an additional accumulator parameter. Choose any to your liking.

Lastly, your buildlist builds a descending list towards 2, and listupto just reverses it; instead move the former into the latter and have it build the correct list right away from 2 up to the upper limit, held by listupto's argument:

(define (listupto m)
  (let buildlist ((n 2))
    (if (> n m) ...
      (cons n (buildlist ...)))))

Named let is described e.g. here.


Update dictionary in constant time in Python Consider a large list of 4 character strings.

For example: ['P0BH', 'LF3J', 'MA1Y', 'STSM', '8Y74', 'JWBD']

I want to add a new string in the following precise manner:

If the new string already exists in the given list, delete the old value from the list and prepend the new string to the front of the list. For example, if the string 'MA1Y' was added, then the output list would look like this:
['MA1Y', 'P0BH', 'LF3J', 'STSM', '8Y74', 'JWBD']. If the new string does not exist in the list, then just prepend the new string to the front. For example, if the string 'FSH7' was added, then the output list would look like this: ['FSH7', 'P0BH', 'LF3J', 'MA1Y', 'STSM', '8Y74', 'JWBD'].

There is one other caveat: the algorithm must run in constant time complexity. I don't care about space complexity.

Naturally, I have already implemented a solution, which is given below. I believe it runs in constant time complexity, but I would like that we improve it or redesign something better. My understanding is that a map has constant time deletion, membership testing and assignment.

Does the addTicker method in my tickerMap class run in constant time?

If my algorithm is poor, please let me know.

import random
import string

class tickersMap:
    def __init__(self, tickersList):
        self.Map = {t:i for i, t in enumerate(tickersList)}
        self.firstIndex = 0

    def addTicker(self, newTicker):
        if newTicker in self.Map:
            print('exists')
            del self.Map[newTicker]
            self.Map[newTicker] = self.firstIndex - 1
            self.firstIndex -= 1
        else:
            self.Map[newTicker] = self.firstIndex - 1
            self.firstIndex -= 1

    def listTickers(self):
        # This is O(n) but is only required for printing
        return sorted(self.Map, key=self.Map.get)

if __name__ == "__main__":

    tickersList = [''.join(random.choice(string.ascii_uppercase + string.digits) for _ in range(4)) for __ in range(6)]

    newTickers = [''.join(random.choice(string.ascii_uppercase + string.digits) for _ in range(4)) for __ in range(3)]

    newTickers.insert(0,tickersList[3])
    newTickers.insert(3,tickersList[2])

    print('Starting ticker list:', tickersList)
    tMap = tickersMap(tickersList)

    for newTicker in newTickers:

        print('\nCurrent map:', tMap.Map)

        print('Want to add:', newTicker)

        tMap.addTicker(newTicker)

        print('New map', tMap.Map)

        newTickersList = tMap.listTickers()

        print('New ticker list:', newTickersList)

You can accomplish this using the standard collections.OrderedDict. According to the docs:

  Deleting an entry and reinserting it will move it to the end.

By default, the presentation will be in insertion order. For you, that is reversed, so you will want to reverse the ordering when you iterate (using .keys() or .items()).

Update

To use the ordered dictionary, you'll have to import collections (unless you're using python3.6, where I think dictionaries become ordered by default- check that).

So:

import collections

od = collections.OrderedDict()

def insert(ticker, price):
    '''
    Insert a ticker->price pair. If ticker is already stored, move it
    to the end (last updated).
    '''
    if ticker in od:
        del od[ticker]

    od[ticker] = price

def each_ticker():
    '''Return ticker symbols in reverse order of insertion'''
    return reversed(od.keys())

def each_ticker_price():
    '''Return (ticker, price) pairs in reverse order of insertion'''
    return reversed(od.items())

insert('AAPL', 100)
insert('MSFT', 200)
insert('GOOG', 300)
insert('AAPL', 400)
insert('MSFT', 350)

for name, price in each_ticker_price():
    print(name, "trading at", price)

Output is:

$ python test.py
MSFT trading at 350
AAPL trading at 400
GOOG trading at 300


Replacing Python Classes with Modules I try to avoid using classes in Python as much as possible; if I don't plan on building on it, I don't build it in the first place. It helps me avoid Java-like classes like FileDownloader(), when I could build download_file() instead.

That being said, I wanted to discuss getter and setter methods -- particularly getter methods. Instead of using class methods, I typically make them module methods (and use the module as others might use a class).

For example:

package/
|
| __init__.py
| constants.py
| main.py
| helpers.py

How this might be used:

#constants.py
import os
def get_default_directory():
    return os.path.join(os.expanduser('~'), 'Desktop', 'files')
def get_server_name():
    return '127.0.0.1'
def get_url_template(site, timezone):
    ...

And so on. This is the go here and change stuff if something changes area of the code.

For helpers.py, you might have some convenience functions that work well as one liners here and there, but duplicating them in each script might be a stretch while you're still formulating a final draft.

For instance, I had something like this, but ended up copying it into a second file later on:

def filename_from_path(path):
    return os.path.split(path)[-1]

def trimext(filename):
    return os.path.splitext(filename)[0]

def raw_filename(path):
    return trimext(filename_from_path(path))

This replaced return os.path.splitext(os.path.split(path)[-1])[0], which ended up in a couple of places (and looked ugly).

main() simply holds all of my high level convenience functions, my "dumb functions", as they were. They just carry out a task, and make it take the least amount of keystrokes possible. Like a main method, but you have many different arrangements available to you.

#main.py
import constants
import helpers

def admin_dry_run(safe_mode=True):
    do_main_a(safe_mode)
    do_main_b(safe_mode)
    do_main_c(safe_mode)

def dry_run(safe_mode=False):
    if safe_mode:
        #even this code could be defined in `helpers` to use
        #the login credentials in `constants`, saving space here
        if helpers.login(constants.get_credentials()):
            do_main_a()
        else:
            do_main_b()
    else:
        do_main_c()

def do_main_a():
    pass #...

Is this typical? Is there anyone who does this, or might be able to suggest a different approach to structuring programs that stretch across multiple files in a similar manner?

So one thing about not using objects is that it makes getters/setters a bit less flexible in that you can't start them out as pure properties and then move them to property() values later.

Also, by using something like helpers.login(..), you're implicitly limiting yourself to one login at a time since the helpers namespace isn't managed like an object namespace would be.  That may or may not be a problem for you now, but it's definitely a corner you may not be aware you're coding yourself into.  If in the future you had two things you wanted to log into using the same code, you'd have to restructure quite a bit.


Swapping pairs of characters in a String Question: Write a program to swap pairs of characters of a String. In
  case of odd number of characters last character should remain
  unchanged.

  Examples:

  Input: HI

  Output: IH

  Input: hello there

  Output: ehll ohtree

My Code:

import java.util.*;
class SwappedPairs
{
    static String swapPairs(String str)
    {
        int len = str.length();
        int n; //array's length
        if(len%2 == 0)
        {
            n = len;
        }
        else
        {
            n=len-1;
        }
        char []arr = new char[n];
        for(int i = 0; i< n; i++)
        {
            arr[i] = str.charAt(i);
        }
        StringBuffer str1 = new StringBuffer(n);
        for(int i = 0; i<=n-2; i= i+2)
        {
            char temp = arr[i];
            arr[i] = arr[i+1];
            arr[i+1] = temp;
            str1 = str1.append(arr[i]).append(arr[i+1]);
        }

        if(len%2 != 0)
        {
             str1 .append(str.charAt(len-1));
        }
        str = str1.toString();
        return str;
    }

    public static void main(String[] args)
    {
        Scanner sc = new Scanner(System.in);
        System.out.println("Enter a string: ");
        String str = sc.nextLine();
        System.out.println(swapPairs(str));
    }
}

Any suggestion on how to improve it? (It gives the desired output)

Using a StringBuffer for building the new string is good,
but the variable name str1 is non-descriptive.

The computation of the required array length can be shortened
with the conditional operator:

int n = len % 2 == 0 ? len : len - 1;

Instead of swapping the array elements you can append them to the
string buffer in opposite order:

for (int i = 0; i <= n - 2; i = i + 2) {
    str1 = str1.append(arr[i+1]).append(arr[i]);
}

(and the re-assignment to str1 is not necessary).

And now one sees that the intermediate array is not needed at all:
You can append the swapped characters directly from the original string:

static String swapPairs(String str) {
    int len = str.length();
    StringBuffer swapped = new StringBuffer(len);
    for (int i = 0; i < len - 1 ; i += 2) {
        swapped.append(str.charAt(i + 1));
        swapped.append(str.charAt(i));
    }
    if (len % 2 != 0) {
        swapped.append(str.charAt(len - 1));
    }
    return swapped.toString();
}


Large-scale pairwise similarity calculations of vectors I am a C++ beginner and am working on a bit of code which calculates the pairwise cosine similarities between vectors. The vectors are TF-IDF vectors and the similarities are used to determine similarity between texts (documents).

My code and approach outlined below work as expected, but runs into performance issues when datasets get larger. The number of vectors often runs into the thousands and the vectors can be quite large themselves (thousands of elements).

I have done the following to try and optimize the code:

In the nested loop, set j equal to i to only calculate similarities one way, i.e. only return results for one side of the diagonal of the similarity matrix;
Passing the vectors as const references to cosine_similarity to avoid the overhead of copying the vectors;
Defined v1i and v2i outside of the loop in cosine_similarity to only look them up once inside the loop (as opposed to using v1[i] and v2[i] in the dot product and sum of squares calculations);
Check for dot_product == 0 in cosine_similarity to avoid magnitude calculation when it is not necessary.

Here's what I have:

// Vector containing all the vectors to compare (populated somewhere else)
std::vector< std::vector<double> > vector_of_vectors;

// Get the number of vectors
int number_of_vectors = vector_of_vectors.size();

// Get the size of a vector (all vectors have the same size)
int vector_size = vector_of_vectors[0].size();

// Vector to store the results
std::vector<double> similarities;

// Nested loop for similarity calculation
for (int i=0; i<number_of_vectors; ++i) {
    for (int j=i; j<number_of_vectors; ++j) {
        if (i == j) {
            similarities.push_back(1);
        } else {
            similarities.push_back(cosine_similarity(vector_of_vectors[i], vector_of_vectors[j], vector_size));
        }
    }
}

double cosine_similarity(const std::vector<double>& v1, const std::vector<double>& v2, const int& vector_size) {
    // Cross-vector dot product
    double dot_product = 0;

    // Sum of squares of first vector
    double ss1 = 0;

    // Sum of squares of second vector
    double ss2 = 0;

    double v1i;
    double v2i;
    for (int i=0; i<vector_size; ++i) {
        v1i = v1[i];
        v2i = v2[i];

        dot_product += (v1i * v2i);
        ss1 += (v1i * v1i);
        ss2 += (v2i * v2i);
    }

    if (dot_product == 0) {
        return 0;
    } else {
        double magnitude = sqrt(ss1 * ss2);
        return dot_product / magnitude;
    }
}

I'm quite happy about how it works, but have the feeling there are some things I could do to make this much faster. Any ideas?

Not so much about performance. The cosine similarity can be expressed in a more concise (and maybe even more performant) way with STL:

dot_product = std::inner_product(v1.begin(), v1.end(), v2.begin());
ss1 = std::inner_product(v1.begin(), v1.end(), v1.begin());
ss2 = std::inner_product(v2.begin(), v2.end(), v2.begin());

This of course assumes that the vectors are of the same size.
Since you want it for every pair of vectors, nothing can be done to speed thins up algorithmically. There are \$O(n^2)\$ pairs, so the performance is bound to be quadratic.
cosine_similarity works with each vector many times, and recomputes its length (ss1/ss2) many times. Compute them all in advance. It is probably the biggest performance gain you can achieve.
There is no need to pass vector_size by reference.


Given a list of words, find the most common initial character and other statistics 20 different words are inputted using a string array.

A C++ program to display the alphabet which has occurred the maximum number of times as starting letter of a word in the list,and how many times it has occurred.

In this program i have also displayed a few other things too,like the longest length,shortest length,average length,maximum number of vowels,letter ending with s.

I would like to know how this program can be written more 'intelligently'.

#include<iostream.h>
#include<string.h>
int main()
{
int i,n,len=0,sum=0,j,max,min,c=0,c2=0,k=0;
int maxo=0,len1,maxi=0,c1=0,len2,counter=0,maxo1=0,len3;
 float avg;
char list[100][100] = { 0 };
char vow[] = "AEIOUaeiou";
char letter;
for(i=0;i<2;i++)
{
cout<<"Enter word:  ";
gets(list[i]);

len=strlen(list[i]);
sum=sum+len;
cout<<"Length of word:  "<<len<<endl;
if(list[i][len-1]=='s')
{cout<<"The Word "<<list[i]<<" ends with s"<<endl;
 c2++;
 }

}
//Word input by user.Prints word along with length.
min=strlen(list[0]);
max=strlen(list[0]);
//Initialising max and min.
for(i=0;i<2;i++)
 {
   if(strlen(list[i])<min)
   {min=strlen(list[i]);}
    if(strlen(list[i])>max)
    {max=strlen(list[i]);}

     }
  for(i=0;i<2;i++)
  {
  if(max==strlen(list[i]))
  cout<<"The max value of the lengths stored:"<<list[i]<<endl<<"Word count:"
  <<max<<endl;
  if(min==strlen(list[i]))
  cout<<"The min value of the lengths stored:"<<list[i]<<endl<<"Word count:"
  <<min<<endl;
  }
//Max and Min value of string lengths are printed.
 avg=sum/2;
 cout<<"Avg length:"<<avg<<endl;
 //Average value printed.
 cout<<"The number of words with s:"<<c2<<endl;
//Word ending with s.

 for (i = 0; i <2; i++)

 {len1 = strlen(list[i]);
 for (k = 0; k < len1; k++)
 {
    for (j = 0; j < strlen(vow); j++)
        //if (list[j][k] == vow[j])
        if (list[i][k] == vow[j])
            c++;
}
cout << "Number of vowels in line " << i << ": " << c << '\n';
if (c>maxo) maxo = c;
c = 0;
cout << "Maximum Vowel count so far:" << maxo << "\n\n";

 cout << "Maximum Vowel count:" << maxo << endl;
   }

 //Vowel maximum count

 for(i = 0 ;i < 2 ;i++)
 { len3 = strlen(list[i]);
    letter = list[i][0];
 {for(j=0;j<len3;j++)
  if(list[i][j]==letter)
   counter++;

 }
 cout << "Number of identical letters as  first letter in line " << i << ": " << counter << '\n';
if (c>maxo1) maxo1 = counter;
counter = 0;
cout << "Maximum letter count so far:" << maxo1 << "\n\n";

 cout << "Maximum letter count:" << maxo1 << endl;
   }

fflush(stdin);
getchar();
return 0;
}

Here are things I see:

Old Headers

#include<iostream.h>

iostream.h should be replaced with the modern day iostream. You can also consider replacing string.h with cstring. When you do this, you have to prefix all functions/classes from the standard library with std::. using namespace std; may seem shorter, but it is best avoided.

Indentation

int main()
{
int i,n,len=0,sum=0,j,max,min,c=0,c2=0,k=0;

You don't seem to have any sort of indentation set up in your main function. Pick some length, such as 4 spaces or a tab, and use it consistently. Indentation makes code easier to read.

Waaaaaaay too many variables in one place

int i,n,len=0,sum=0,j,max,min,c=0,c2=0,k=0;
int maxo=0,len1,maxi=0,c1=0,len2,counter=0,maxo1=0,len3;

This honestly looks horrible at first glance. Declaring variables at the beginning of a function is not something C++ requires. I suggest you move all of these variables close to where they are used, and use initialization instead of separate declaration and definition. For example:

len=strlen(list[i]);

Can be replaced with:

std::size_t len=strlen(list[i]);

Usage of C Style Multidimensional Arrays

char list[100][100] = { 0 };

C style multidimensional arrays are error prone, don't know their own size, and can easily decay to pointers. As such, they are best avoided. Consider using a std::arrays of std::strings, which are more versatile and less error prone.

Usage of C Style Strings

char vow[] = "AEIOUaeiou";

If you are using C++, then why are you using C style strings? Switch over to the much better std::string.

NEVER use gets

gets(list[i]);

This Stack Overflow answer tells you all you need to know. When you start using std::string, then use getline to read in a string.

Don't use endl unless you want to flush the output

... << endl;

Most people don't know when starting C++ that endl unnecessarily flushes the output. Switch over to \n.

Don't use strlen as a loop condition

for (j = 0; j < strlen(vow); j++)

Unless your compiler knows to optimize this out, strlen might be called every iteration in this code. To avoid this, put the length in its own variable.

Better Variable Names

Variable names such as i and c should only be reserved for loop indices. Use better variable names that clarify your intent.

fflush(stdin)

fflush(stdin)

https://stackoverflow.com/a/18170435/6525260. Don't play with undefined behavior.

getchar

In C++, a better way to wait for a character is:

std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');


Testimonial section CSS inheritance and proper use of HTML5 elements I created this testimonial section as a prototype.

HTML

This was pretty good.

      – Bobby, Jersey City, NJ

This is pizza is the most bearable thing I ever had in Joisey. But it still sucks, just like everyone from there.

      – Vinny, New York, NY

I really savored the smoky undertones of the imported sausage. The cheese had a wonderful texture. It paired well with my pinot noir in a dance of sensations.

      – Alex, San Francisco, CA

CSS

.testimonials {
  width: 720px;
  max-width: 96%;
  margin: 0 auto;
}
.testimonials blockquote {
  background-color: #fff;
  border-left: 4px #61acca solid;
  font-size: 21px;
  line-height: 1.6;
}

.testimonials blockquote {
  padding: 10px 20px;
}

.testimonials blockquote {
  background-image: url('http://shared-assets.s3.amazonaws.com/codepen/img/elements/quotes/quote-999.jpg');
  background-repeat:no-repeat;
  background-size: 33px 45px;
  background-position: 10px 5px;
}

.testimonials cite {
  font-size: 0.7em;
  font-style: normal;
}
.author {
  font-weight: bold;
  font-size: 1.3em;
  margin: 0 6px 0 36px;
}
p { margin: 0 0 0 36px; }

Everything looks clean. However, I found some things though:

font-size - Make the units consistent. You use px and em. I suggest you go em all the way.
background-size - Well, it exists for that reason, but it would be better if you actually sized the image in those dimensions rather than having them resized in CSS.
You "namespaced" everything by prefixing .testimonials. That's being a good developer. You're avoiding your styles from clobbering other elements. However, you forgot to do that for .author and p.


C++ Merge Sort Implementation - Pt2 Following the review of my old merge sort implementation here, it appears that the biggest improvement to make was to use iterators instead of copying the vectors. Having never used iterators before, I'd like a review of the new code following that review:

#include <iostream>
#include <vector>
#include <algorithm>

template<typename T>
void Merge(T, T, T, T);
template<typename T>
void MergeSort(T, T);

int main()
{
    // As last time, this is just test code and does not need to be reviewed, but
    // is included for completeness
    std::vector<int> a;
    for (int i = 0; i < 10; i++) {
        a.push_back(10.0 * rand() / RAND_MAX);
    }
    for (int i = 0; i < a.size(); i++) {
        std::cout << a[i] << " ";
    }
    std::cout << "\n";
    // Note this swap is now in place, previously it returned a new vector
    MergeSort(a.begin(), a.end());
    for (int i = 0; i < a.size(); i++) {
        std::cout << a[i] << " ";
    }
    std::cout << "\n";
}

template<typename T>
void Merge(T arr1Begin, T arr1End, T arr2Begin, T arr2End) {
    T i1 = arr1Begin;
    T i2 = arr2Begin;
    int i = 0;
    std::vector<int> res(arr2End - arr1Begin, 0);
    while (i1 < arr1End && i2 < arr2End) {
        if (*(i1) < *(i2)) {
            res[i++] = *(i1);
            std::advance(i1, 1);
        }
        else {
            res[i++] = *(i2);
            std::advance(i2, 1);
        }
    }
    while (i1 < arr1End) {
        res[i++] = *(i1);
        std::advance(i1, 1);
    }
    while (i2 < arr2End) {
        res[i++] = *(i2);
        std::advance(i2, 1);
    }
    std::copy(res.begin(), res.end(), arr1Begin);
}

template<typename T>
void MergeSort(T arrBegin, T arrEnd) {
    int size = arrEnd - arrBegin;
    if (size < 2) {
        return;
    }
    if (size == 2) {
        if (*arrBegin > *(arrEnd - 1)) {
            std::swap(*arrBegin, *(arrEnd - 1));
        }
        return;
    }
    MergeSort(arrBegin, arrBegin + size / 2);
    MergeSort(arrBegin + size / 2, arrEnd);
    return Merge(
        arrBegin, arrBegin + size / 2,
        arrBegin + size / 2, arrEnd
    );
}

My questions this time:

I'm dereferencing a lot of iterators in order to swap and assign values, is there a better way to be doing this?
Is this implementation better than my last implementation? I'm looking for answers that think about stability,
efficiency in terms of speed/memory, etc. (Possibly an obvious question)
Also, I know nothing about templates, is there anything more to it than adding the line template<typename T> then line before the function and then using T within it?
I can't figure out how to get rid of std::vector<int> res(arr2End - arr1Begin, 0);. Obviously this goes against the templating, how should I store the results while I'm doing the merge? (Edit: since looking back at the previous accepted answer, I figure I should create a new vector outside the original call to MergeSort (inside main) and pass the .begin() of that array as the destination, rather than defining a new array inside each Merge() call and using std::copy. Is this correct?)

Edit: Thanks to @ratchetfreak for pointing out a bug whereby I was overwriting elements of arr1 without storing the overwritten element elsewhere beforehand (essentially losing it).

Well, the first point is that you are still allocating memory for every single merge. That's expensive.

Second, you are always copying all elements. That's inefficient too.

Third, prefer to move instead of copying. Some types are move-only, others just far more efficiently moved.

Try something along these lines:

#include <iterator>
#include <algorithm>
#include <utility>
#include <memory>

template<class It>
void MergeSortMerge(It begin, It mid,
    typename std::iterator_traits<It>::value_type* aux)
{
    while(begin != mid && !(*mid < *begin))
        ++begin;
    if(begin != mid)
        std::move(begin, mid, aux);
    for(; begin != mid; ++begin) {
        if(*mid < *aux) {
            *begin = std::move(*mid);
            ++mid;
        } else {
            *begin = std::move(*aux);
            ++aux;
        }
    }
}

template<class It>
void MergeSortImpl(It begin, It end,
    typename std::iterator_traits<It>::value_type* aux)
{
    const auto size = std::distance(begin, end);
    if(size < 2)
        return;
    const auto mid = std::next(begin, size / 2);
    MergeSortImpl(begin, mid, aux);
    MergeSortImpl(mid, end, aux);
    MergeSortMerge(begin, mid, aux);
}

template<class It>
void MergeSort(It begin, It end) {
    const auto size = std::distance(begin, end);
    if(size < 2)
        return;
    using T = typename std::iterator_traits<It>::value_type;
    std::unique_ptr<T[]> aux = new T[size / 2];
    MergeSortImpl(begin, end, aux.get());
}

The one style-comment I have is that streaming a single character as a char is marginally more efficient than as a length-1-string.

You only use arrEnd - 1 in two places, where you could use arrBegin + 1 instead. Changing that allows the use of mutable forward-iterators, whereas before you required mutable bidirectional iterators.

Adressing your extra questions:

It's better to move than copy where you can.
Your algorithm is still not stable, change if (*(i1) < *(i2)) to if (!(*i2 < *i1)).
You have fewer allocations than before, one for every merge instead of one for every recursive call to sort. That's still far more than the single one needed though.
That's the beginning. Next, you have to really think about how to most generically use the type / expressions affected, and to minimize the interface used.

std::iterator_traits, std::distance and std::advance are your friend there, and it wouldn't be amiss to only use < and == out of all relational operators, as those are the ones used by most generic code (especially all of the standard library).
Nearly. Use an extra-level of indirection, MergeSort is the public API doing the allocating and calling the recursive MergeSortImpl which calls MergeSortMerge.


Command line flags & sorting algorithms I've made my first-ever program in Go, which sorts a given input of integers. There are 2 options: The list of unsorted integers and the algorithm to use.

Since it's my first go project I'm a bit unsure of a few things. For example, the "floating" variable declarations.

I think that the algorithms itself are okay, therefore I only post the executable program:

package main

import (
    "fmt"
    "flag"
    "strings"
    "strconv"
    "os"
    "github.com/afroewis/go-sorting-algorithms/sortalgos"
)

func main() {
    inputFlag := flag.String("input", "", "Input numbers to sort. Example: 10 20 5 1003 94 928")
    algoFlag := flag.String("algo", "", "Sorting algo to use. Available values: bubble, selection, insertion. ")
    flag.Parse()

    // Parse input
    input := strings.Split(*inputFlag, " ")
    var numbers [] int

    for i := 0; i < len(input); i++ {
        val, err := strconv.Atoi(input[i])

        if err != nil {
            panic(err)
        }

        numbers = append(numbers, val)
    }

    var result []int;

    switch *algoFlag {
    case "bubble":
        result = sortalgos.BubbleSort(numbers)
    case "selection":
        result = sortalgos.SelectionSort(numbers)
    case "insertion":
        result = sortalgos.InsertionSort(numbers)
    case "":
        fmt.Println("Error: -algo flag not set. For usage infos use ./main -help")
        os.Exit(1)
    }

    fmt.Println(result)
}

I recommend looping through slice with range syntax.

for _, v := range(input) {
    val, err := strconv.Atoi(v)

    if err != nil {
        panic(err)
    }

    numbers = append(numbers, val)
}

You can replace case "" with default.

Besides that, it looks OK to me.


Function for approximating a sine signal I want to approximate a sine signal with a parabola, which is given by the function:

$$f(x) = 0.4053 \cdot x \cdot (3.1415 - x) \space\text{for}\space 0 \le x \le 3.1415$$

In a first approach, I wrote the following code which works fine:

float sin1(int phase)
{
    if(phase % 1000 <= 500)
    {
    return(0.4053*2*3.1415*(phase % 1000)/1000*(3.1415 - 2*3.1415*(phase % 1000)/1000));
    }
    else
    {
    return(-0.4053*2*3.1415*(phase % 500)/1000*(3.1415 - 2*3.1415*(phase % 500)/1000));
    }
}

As I want to use this function on a microcontroller, I'm interested in how to optimize this code regarding memory consumption and performance.

how to optimize this code regarding memory consumption and performance. (?)

Consider redefining the angle measurement.  It appears int phase is 0.36 degrees.  A natural choice would be 1024 phase to 1 revolution.  Now the problem becomes one using a binary angle measure. Not only does this simplify small things like scaling, it opens up coding choices  as there are many trig function code bases designed withs BAMS, Binary Angular Measurement System.
Code uses double constants and so makes for likely slower computation on an embedded system.  If all that is needed is float, uses float constants and float math.

// return(0.4053*2*3.1415*(phase % 1000)/1000*(3.1415 - ...
return(0.4053f*2*3.1415f*(phase % 1000)/1000*(3.1415f ...

OP reports phase guaranteed to be positive. yet it is minor to adjust code for full int range.

// if(phase % 1000 <= 500)
phase %= 1000;
if (phase < 0) phase += 1000;
if (phase <= 500)

Accuracy: Using 3.1415 for machine_pi can be improved at no computational cost by using  #define MACHINE_PI 3.141592653589793f.  This will improve the end points in the 5th or 6th decimal place.  Use 0.405284734572655f instead of 0.4053 to insure sine(90° (250 phase)) is 1.0f.
If your processor lacks a built-in FP hardware, I recommend a fast integer only solution.  e.g. CORDIC or a small linear interpolation should only low precision be needed.

Using a parabola model like OP's  (1 cycle = 1024 BAM).  The below code is effectively the same preciseness as OP's code with only integer math: and, or, +, -, *, negate and shift.

Input  range [0R 2πR]  mapped to [0 to 1024]
Output range [-1.0 1.0] mapped to [-16,384 16,384]

int_fast16_t sin_bam(int angle_bam) {
  angle_bam %= 1024;
  if (angle_bam < 0)
    angle_bam += 1024;
  uint_fast16_t sub_angle = angle_bam % (1024u / 2);
  int_fast16_t sn = unsigned_mult16x16_into_32(sub_angle, (1024/2 - sub_angle)) / 4;
  if (angle_bam & 512) {
    sn = -sn;
  }
  return sn;
}


SQLite and Python: commit once every 10 seconds maximum, and not after every client request In a Python Bottle server using SQLite, I noticed that doing a DB commit after each INSERT is not efficient: it can use 100ms after each client request. Thus I wanted to improve this in How to commit DB changes in a Flask or Bottle app efficiently?
.

I finally came to this solution, which is more or less a "debounce"-like method: if multiple SQL INSERT happen during a 10-second timeframe, group all of them in a single DB commit.

What do you think of the following code? Is it safe to do like this?

(Note: I know that the use of a global variable should be avoided, and replaced by a class / object with attributes, etc. I'll do this, but this part is not really on topic here).

import bottle, sqlite3, random, threading, time

@bottle.route('/')
def index():
    global committhread
    c = db.cursor()
    c.execute('INSERT INTO test VALUES (?)', (random.randint(0, 10000),))
    c.close()
    if not committhread:
        print('Calling commit()...')
        committhread = threading.Thread(target=commit)
        committhread.start()
    else:
        print('A commit is already planned.')
    return 'hello'

def commit():
    global committhread
    print("We'll commit in 10 seconds.")
    time.sleep(10)  # I hope this doesn't block/waste CPU here?
    db.commit()
    print('Committed.')
    committhread = None

db = sqlite3.connect('test.db', check_same_thread=False)
db.execute("CREATE TABLE IF NOT EXISTS test (a int)")
committhread = None
bottle.run(port=80)

If you run this code, opening http://localhost/ once will plan a commit 10 seconds later. If you reopen http://localhost/ multiple times, less than 10 seconds later, you will see that it will be grouped in the same commit, as desired.

Note: this method is not "Do it every 10 seconds" (this would be a classic timer), but rather: "Do it 10 seconds later; if another INSERT comes in the meantime, do all of them together". If there is no INSERT during 60 minutes, with my method it won't do anything at all. With a timer it would still periodically call a function (and notice there's nothing to do).

Worth reading too: How to improve SQLite insert performance in Python 3.6?

What do you think of the following code? Is it safe to do like this?

In my opinion, it is not. So many things could go wrong.

One example: in this code there is no exception handling. If your program crashes for any reason, your routine may not be triggered. Fix: add an exception handler that does some cleanup, commits and closes the DB. Or better yet, just do that commit in the finally block.

By the way the doc says this about the close function:

  This closes the database connection. Note that this does not
  automatically call commit(). If you just close your database
  connection without calling commit() first, your changes will be lost!

So it is a good idea to commit systematically.

  time.sleep(10)  # I hope this doesn't block/waste CPU here?

time.sleep blocks the calling thread. It is useless anyway because what you want here is a timer, not a thread. You can have a timer routine that runs every n seconds to perform a  given task. But you should still have a commit in the finally block, so that all pending changes are written to the DB when the program ends, even after an exception.

Now to discuss the functionality more in depth:
You say:

  In a Python Bottle server using SQLite, I noticed that doing a DB
  commit after each INSERT is not efficient: it can use 100ms after each
  client request.

That may not be 'efficient' but if I have to choose between slow and safe there is no hesitation. Have you actually measured how long it takes on your own environment ?

On Stack Overflow you wrote:

  I optimized my server to serve pages very fast (10ms), and it would be
  a shame to lose 100ms because of DB commit.

While I applaud your obsession with performance, does 100 ms really make a difference to your users ? It normally takes more than 100 ms to load a page or even refresh a portion of it using Ajax or a websocket. The latency resides in the network transport. I don't know how your application is structured but my priority would be to deliver as little traffic as possible to the users. Websocket + client-side JS should do.

Perhaps using a different storage medium could improve IO performance. If you are not using a SSD drive, maybe you could consider it or at least test it.

Before writing code like this I would really try to exhaust all possibilities, but it is better (more reliable) to let SQLite handle things using the options that already exist. What have you tried so far ?

Would this be acceptable to you ?

PRAGMA schema.synchronous = OFF

  With synchronous OFF (0), SQLite continues without syncing as soon as
  it has handed data off to the operating system. If the application
  running SQLite crashes, the data will be safe, but the database might
  become corrupted if the operating system crashes or the computer loses
  power before that data has been written to the disk surface. On the
  other hand, commits can be orders of magnitude faster with synchronous
  OFF.

Source: PRAGMA Statements

There is a risk of corruption in case of power loss but this is no worse than what you are doing. If on the other hand data integrity is more important you should stick to a full commit after every operation.

You should also have a look at Write-Ahead Logging. This may interest you if there are concurrent writes to your database. Otherwise opening the DB in EXCLUSIVE mode may bring some benefits (see the PRAGMA page for details).

More detailed discussions:

Atomic Commit In SQLite
35% Faster Than The Filesystem

Last but not least: transactions. SQLite starts an implicit transaction automatically every time you run a SQL statement and commits it after execution. You could initiate the BEGIN & COMMIT TRANSACTION statements yourself. So if you have a number or related writes, regroup them under one single transaction. Thus you do one commit for the whole transaction instead of one transaction per statement (there is more consistency too: in case an error occurs in the middle of the process you won't be left with orphaned records).

There are quite many things you can try until you find the mix that is right for you.


Function to censor single word in a sentence The following function censors a single word of choice in a sentence with asterisks, regardless of how many times the word appears. How can I further simplify this code?

def censor(text,word):
  text_list = text.split()
  for i in range(0,len(text_list)):
    if text_list[i] == word:
      text_list[i] = '*' * len(text_list[i])
    else:
      continue
  return" ".join(text_list)

As mentioned in the comments, you could use str.replace() to replace all occurrences of a certain substring:

def censor(text, word):
   return text.replace(word, "*"*len(word))

To ensure that only entire words are replaced, though, you should use regular expressions:

import re

def censor(text, word):
    pattern = r"\b" + re.escape(word) + r"\b"
    return re.sub(pattern, "*"*len(word), text)

Unlike your original code, the regular expression solution also handles words that have punctuation attached.

Some other things to think about:

Your else: continue statement can be omitted;
Your function does not have a docstring. If you plan on making it a public function (as part of an API, for example), you should explain what arguments it expects and what it returns. If you plan on using it internally, at the very least, provide a single-line docstring explaining what the function does.
range() assumes start is 0 if no explicit start is passed, thus you can change it to for i in range(len(text_list)):


Switch statements in Rock-Paper-Scissors game I am very much new to design pattern. I'm trying to modify the Rock, paper, scissor game from an example book where I try to add more various design pattern. But I am encountering two similar switch statement which I think have duplicates and I don't have any idea on removing those duplicates.

A abstract class:

abstract class UserVsComp {
    protected static Item item;
    abstract protected Item getItem() throws WrongMove;
}

Here are the two switch statement:(First Class is for user movement)

class UserMove extends UserVsComp {
    private String move;
    public UserMove(String move) { this.move = move; }
    public Item getItem() throws WrongMove {
        switch (choiceValue(this.move)) {
            case 0 :  item = new Paper(); break;
            case 1 :  item = new Scissors(); break;
            case 2 :  item = new Rock(); break;
            default:  throw new WrongMove("Wrong move. Game end");
        }
    return item;
}

Second class is for Computer movement:

class ItemGenerator extends UserVsComp {
    public Item getItem() throws WrongMove {
        switch((int) (Math.random() * 2)) {
            case 0: item = new Paper(); break;
            case 1: item = new Scissors(); break;
            case 2: item = new Rock(); break;
            default:  throw new WrongMove("Wrong move. Game end");
        }
    return item;
    }
}

The two switch statement have same case (I guess duplicacy). I'd like to know if there is any way to remove this duplicacy.

Instead of creating a new Item each time you can just use a enum:

public enum Move{
    Paper{
        public boolean canBeat(Move other){
            return other==Rock;
        }
    },
    Scissor{
        public boolean canBeat(Move other){
            return other==Paper;
        }
    },
    Rock{
        public boolean canBeat(Move other){
            return other==Scissor;
        }
    };

    public abstract boolean canBeat(Move other);

    public boolean losesTo(Move other){
        return other.canBeat(this);
    }
}

Then You can keep them in an array (like what is returned by Move.values()) and index into that.

public Item getItem() throws WrongMove {
    int choice = choiceValue(this.move);
    if(choice<0||choice >= Move.values().length)
        throw new WrongMove("Wrong move. Game end");
    else return  Move.values()[choice];
}


Check value existence in two lists and gather the result in a csv I have two lists of protein sequences, I have to check every entry's existence in the two lists, say like

list A = [1,2,3,4]
list B= [3,4,5]

result = [
[1, true, false],
[2, true, false],
[3, true, true],
[4, true, true],
[5, false, true]
]

I did it this way. Not sure if it's the right way

def FindDifferences():
    df1 = pd.read_csv('Gmax_v6_annotation_info.txt', names=['name'], usecols=[0], delimiter='\t')
    df2 = pd.read_csv('Gmax_v9_annotation_info.txt', names=['name'], usecols=[2], delimiter='\t')
    v6_set = set(df1['name'])
    v9_set = set(df2['name'])
    result = []
    for val in v6_set:
        if val in v9_set:
            result.append([val, True, True])
        else:
            result.append([val, True, False])
    for val in v9_set:
        if val not in v6_set:
            result.append([val, False, True])
    result_df = pd.DataFrame(result, columns=['name', 'inv6', 'inv9'])
    result_df.to_csv('result_csv.csv', index=False, header=False)
    return

General problem

It's not much, but for your general problem, mathematically speaking, I'd split the logic into three steps (It might be a bit more code, but personally speaking, it keeps things cleaner):

I'd intersect list_A with list_B:

def intersect(list_A, list_B):
    return [[x, True, True] for x in set(list_A).intersection(list_B)]

Which is going to give you:

  [[3, True, True], [4, True, True]]

Then, I'd do list_A - list_B:

def a_diff_b(list_A, list_B):
    return [[x, True, False] for x in set(list_A).difference(list_B)]

Which is going to give you:

  [[1, True, False], [2, True, False]]

Then, I'd do list_B - list_A:

def b_diff_a(list_A, list_B):
    return [[x, False, True] for x in set(list_B).difference(list_A)]

Which is going to give you:

  [[5, False, True]]

Intersection

  In mathematics, the intersection A ∩ B of two sets A and B is the set
  that contains all elements of A that also belong to B (or
  equivalently, all elements of B that also belong to A), but no other
  elements.

Complement

  In set theory, the complement of a set A refers to elements not in A.
  The relative complement of A with respect to a set B, also termed the
  difference of sets A and B, written B ∖ A, is the set of elements in B
  but not in A. When all sets under consideration are considered to be
  subsets of a given set U, the absolute complement of A is the set of
  elements in U but not in A.


Repository for data retrieval I have created a really simple repository for some simple data retrieval and was wondering if i was going in the right direction.  Has anyone got any thoughts on this?

interface

public interface IRepository<T> where T : class {
    IEnumerable<T> GetAll();
    T GetById(int id);
    IEnumerable<T> Filter(Expression<Func<T, bool>> filter);
    void Dispose();
}

class

public class JobRepository : IRepository<Job>, IDisposable{
    /// <summary>
    /// handles all job data retrieval
    /// crude but will suffice for now
    /// </summary>
    CentralRepositoryContainer _context = null;

    public JobRepository() {
        CentralRepositoryContainer context =
            HttpContext.Current.GetCentralRepositoryContext();
    }

    /// <summary>
    /// returns all jobs
    /// </summary>
    /// <returns>IEnumerable<Job></returns>
    public IEnumerable<Job> GetAll() {
        try {
            return this._context.Jobs;
        } catch (SqlException ex) {
            //error so dispose of context
            this.Dispose();
            //wrap and rethrow back to caller
            throw new CentralRepositoryException("Error getting all jobs", ex);
        }

    }

    /// <summary>
    /// return job
    /// </summary>
    /// <param name="id">integer to search on</param>
    /// <returns>job</returns>
    public Job GetById(int id) {
        try {
            return this._context.Jobs.SingleOrDefault(j => j.JobId == id);
        } catch (SqlException ex) {
            //error so dispose of context
            this.Dispose();
            //wrap and rethrow back to caller
            throw new CentralRepositoryException("Error getting all job by id", ex);
        }
    }

    /// <summary>
    /// filters based on parsed filter expression
    /// </summary>
    /// <param name="filter">filter to search on</param>
    /// <returns>IQueryable<Job></returns>
    public IEnumerable<Job> Filter(System.Linq.Expressions.Expression<Func<Job, bool>> filter) {
        try {
            return this._context.Jobs.Where(filter);
        } catch (SqlException ex) {
            //error so dispose of context
            this.Dispose();
            //wrap and rethrow back to caller
            throw new CentralRepositoryException("Error getting all job by filter", ex);
        }
    }

    public void Dispose() {
        if (this._context != null) {
            this._context.Dispose();
        }
    }
}

Especially in such simple cases you don't want to create a repository over an EF (I assume it's EF -- but the same goes for Linq2SQL) context.

The DbContext basically is an implementation of the repository pattern. Your repository on top just creates another layer of complexity without actually providing any serious benefit.

In more complicated cases wrapping the DbContext with a specific interface may make sense of course.


Pick the best combination of n choose k models Algorithm

  Let \$M_0\$ denote the null model which contains no predictors. This model simply predicts the sample mean of each observation.

  For \$k=1,2,\ldots,n\$:

  Fit all \$n \choose k\$ models that contain exactly \$k\$ predictors.
  Pick the best among these \$n \choose \$ models, and call it \$M_k\$. Here the best is defined as having the smallest RSS or equivalent measure.

  Select the single best model among \$M_0,M_1,\ldots,M_n\$ using cross validated prediction error, \$C_p\$, BIC, \$R^2_{\mathit{adj}}\$ or any other method.

I looked up a tutorial and created a function based upon its script. It's essentially used so I can select dependent variables that's a subset of a data frame. It runs but it is very very slow.

How would I flatten a nested for-loop such as this?

# Loop over all possible combinations of k features
for k in range(1, len(X.columns) + 1):
       # Looping over all possible combinations: from 11 choose k
       for combo in itertools.combinations(X.columns,k):
           # Store temporary results
           temp_results = fit_linear_reg(X[list(combo)],Y)

           # Append RSS to RSS Lists
           RSS_list.append(temp_results[0])

I tried implementing an enumerate version but it did not work. I'm not sure, how I can flatten the nested for loop, such that I can append the results of a function to a list.

# This function takes in a subset of a dataframe representing independent
# variables (X) and a column for dependent variable (Y). This function fits
# separate models for each possible combination of the k predictors (which is
# based on the column length of X) and then select the best subset. The
# resulting output is a dataframe.

def BestSubsetSelection(X,Y):
    # number of predictors
    k = len(X.columns)
    # Store the RSS from a linear regression model
    RSS_list = []
    # Store the R-square from a linear regression model
    R_squared_list = []
    # Store the features for a given iteration.
    feature_list = []
    # Store the number of features used for a given iteration. This corresponds with the feature_list.
    numb_features = []

    # Loop over all possible combinations of k features
    for k in range(1, len(X.columns) + 1):
            # Looping over all possible combinations: from 11 choose k
            for combo in itertools.combinations(X.columns,k):
                # Store temporary results
                temp_results = fit_linear_reg(X[list(combo)],Y)

                # Append RSS to RSS Lists
                RSS_list.append(temp_results[0])

                # Append R-Squared TO R-Squared list
                R_squared_list.append(temp_results[1])

                # Append Feature/s to Feature list
                feature_list.append(combo)

                # Append the number of features to the number of features list
                numb_features.append(len(combo))

    df = pd.DataFrame({
        'No_of_Features': numb_features,
        'RSS' : RSS_list,
        'R-Squared' : R_squared_list,
        'Features' : feature_list
    })

    # Finding the Best Subsets for each number of features

    # The smallest RSS
    df_min = df[df.groupby('No_of_Features')['RSS'].transform(min) == df['RSS']]
    # The Largest R-Squared Value
    df_max = df[df.groupby('No_of_Features')['R-Squared'].transform(min) == df['R-Squared']]
    display(df_min)
    display(df_max)

    # Adding columns to the dataframe with RSS and R-Squared values of the best subset
    df['min_RSS'] = df.groupby('No_of_Features')['RSS'].transform(min)
    df['max_R_Squared'] = df.groupby('No_of_Features')['R-Squared'].transform(max)

This code is taken from my IPython notebook.

comments

comments should explain why you do something, not what you do. # Append R-Squared TO R-Squared list adds nothing of value. On the contrary, it uses vertical space, and if ever you change something of the code you will need to change the coàmments as well

# This function takes in a subset of a dataframe representing independent
# variables (X) and a column for dependent variable (Y). This function fits
# separate models for each possible combination of the k predictors (which is
# based on the column length of X) and then select the best subset. The
# resulting output is a dataframe.

could be the docstring of the method if it were correct. This function does not return a DataFrame, but only prints the results

functions

Now you have 1 monster function that:
- calls the test
- aggregates the values
- displays the results

Better would be to split this

getting the results

instead of having to append the results to 4 lists, I would extract this to a generator that yields a dict and then use something like  DataFrame.from_records to combine this.

powerset

what you do here:

for k in range(1, len(X.columns) + 1):
        # Looping over all possible combinations: from 11 choose k
        for combo in itertools.combinations(X.columns,k):

looks a lot like the powerset itertools-recipe, so let's use that one:

from itertools import chain, combinations

def powerset(iterable):
    "powerset([1,2,3]) --> () (1,) (2,) (3,) (1,2) (1,3) (2,3) (1,2,3)"
    s = list(iterable)
    return chain.from_iterable(combinations(s, r) for r in range(len(s) + 1))

def generate_combo_comparisons(X, Y):
    for combo in powerset(X.columns):
        if not combo:  # the first combo is empty
            continue
        RSS, R_squared = fit_linear_reg(X[list(combo)], Y)
        yield {
            "No_of_Features": len(combo),
            "RSS": RSS,
            "R-Squared": R_squared,
            "Features": combo,
        }

To get the maximum indices of each group, you can use groupby.idxmax

You add columns 'min_RSS'  to the original DataFrame. Better here would be to generate a new summary DataFrame

def subset_results(X, Y):
    df = pd.DataFrame.from_records(
        data=list(generate_combo_comparisons(X, Y)),
        index=["No_of_Features", "RSS", "R-Squared", "Features"],
    )

    summary = df.groupby("No_of_Features")["R-Squared"].agg(
        {"RSS": "min", "R-Squared": "max"}
    )
    df_min = df.loc[df.groupby("No_of_Features")["RSS"].idxmin()]
    df_max = df.loc[df.groupby("No_of_Features")["R-Squared"].idxmax()]
    return df, df_min, df_max, summary

And then you can pass these results on to the plotting function


Null Object Pattern for STL Types Background

In C++ unlike other OOP languages, we do not have a Base Class from where All Classes are derived. I understand in MFC and pre STL, some Compilers did had a similar model, so lets leave it out of the discussion and focus on what the standard provides.

In C++11, auto was introduced to derive the type from rvalue just another way of type deduction of templates. This was a huge relief when one has to write long list of types with all template parameters.

Using Naked Pointers were prone to errors so Smart Pointers were introduced as a remedy for the unavoidable evil.

Use Case

I have a function which should return something that would indicate Null Object when no data was found or if it found needs to return the data. I would like to know, if C++11 has an elegant way of writing the below code, considering, the template types are painful to read.

Code

std::unique_ptr<std::tuple<tstring, int, tstring> > CCOMErrorHandler::PopError()
    {
    unique_ptr<std::tuple<tstring, int, tstring> > error = nullptr;
    if (!m_lstErrors.empty())
        {
        error = std::unique_ptr<std::tuple<tstring, int, tstring> >(new std::tuple<tstring, int, tstring>(m_lstErrors.front()));
        if (error)
            {
            m_lstErrors.pop_front();
            }
        }
    return(error);
    }

One option is to use a typedef or macro replacement, but generally am not a big FAN as that keeps polluting namespace though you may have to use to few times in your code often only once.

The first question is, why are you using pointers in the first place here? Allocation with new should only be used when required. Perhaps it is in this case, but I'd rethink your design. Are the errors polymorphic? If so, why? If not, why do you need to use pointers?

Many STL containers use either an error-code approach, that is, they return a std::pair<T, bool>, where the bool represents success/failure, or they deal
directly with iterators, and return an iterator to end() in the case of failure. Depending on the type of m_lstErrors, this could be a possibility (although obviously this won't work for a container adapter like std::stack).

Though it is not contained in the C++ standard library at the moment (it is slated for inclusion in C++14), std::optional was basically made for this purpose. In fact, it can be written using only C++11 features (and there's a reference implementation you could use here). Boost also contains a boost::optional type, although this lacks things such as move semantics and safe (explicit) bool conversion (although other than that, it is very similar in nature). This would change the code to look like this:

std::optional<std::tuple<tstring, int, tstring>> CCOMErrorHandler::PopError()
{
    typedef std::tuple<tstring, int, tstring> tuple_type;
    std::optional<tuple_type> error;
    if(!m_lstErrors.empty()) {
        error = *m_lstErrors.front();
        m_lstError.pop_front();
    }
    return error;
}

cppreference has an API reference for std::optional.

As for the complaint about template types, you can always wrap them in another namespace to help prevent pollution:

namespace mynamespace
{
namespace detail
{
    typedef std::tuple<tstring, int, tstring> error_type;
    typedef std::optional<error_type> optional_error;
}

detail::optional_error CCOMErrorHandler::PopError()
{
    //...
}


Log Retrieval Script I wrote a simple script to retrieve the logs from my website, and place them in a subdirectory called logs. It fetches the logs using sftp. I would like help with the error handling. I'm sure that there are errors I did not see, and I would like to make it so that it fails gracefully.

#!/bin/bash
# gets the logs

# Location to place downloaded logs
PlaceLogs=./logs
# ssh connection to sftp files from
SSHLocation=hjsblog
# Location and pattern of logs
GetLogs=../logs/access*

#######################################
#######################################

CWD=$(pwd)

# move to get location
if [ "$(ls -A $PlaceLogs)" ]
then
    echo $PlaceLogs is not empty
    return
fi
mkdir -p $PlaceLogs
cd $PlaceLogs

# perform the get operation
sftp $SSHLocation << EOF
cd $(dirname $GetLogs)
get $(basename $GetLogs)
EOF

# uncompress files
bunzip2 *.bz2

# return to script dir
cd $CWD

handle more possibilities

Anything touching the file system can fail.

mkdir You could have permission problems or something else.  Do mkdir foo || exit 3.  Feel free to replace the exit with your own function for error handling.
cd is the same deal as mkdir
check for the existance of your directory before lsing it: if [[ -d $PlaceLogs ]]

Also, do any bz2 exist after the sftp?  You should check before bunzip'ing them.

You can safely ignore all of these things too, but since you specifically asked about error handling it seemed good to point them out.  If this is a critical piece of software then I would build a way for these failures to raise alerts with your DevOps team.

shellcheck.net

Run all of your scripts through http://www.shellcheck.net/ until you get in better habits.

use double square brackets

Double square brackets are better than single square brackets.  They have less surprises.  The primary one is that if you have an empty variable you can do

if [[ $FOO == "" ]]

where single brackets require a placeholder so none of the arguments disappear if missing:

if [ x$FOO == "x" ]

Without the xs the $FOO would disappear as a shell argument when empty.


Sudoku grid validator I have coded a sudoku grid validator as a problem set from an online course.

My input to the validator is a list of lists, that represents a sudoku grid. For reference, below are examples of an ill-formed sudoku grid, an invalid one, and a valid one.

# check_sudoku should return None
ill_formed = [[5,3,4,6,7,8,9,1,2],
              [6,7,2,1,9,5,3,4,8],
              [1,9,8,3,4,2,5,6,7],
              [8,5,9,7,6,1,4,2,3],
              [4,2,6,8,5,3,7,9],  # <---
              [7,1,3,9,2,4,8,5,6],
              [9,6,1,5,3,7,2,8,4],
              [2,8,7,4,1,9,6,3,5],
              [3,4,5,2,8,6,1,7,9]]

# check_sudoku should return True
valid = [[5,3,4,6,7,8,9,1,2],
         [6,7,2,1,9,5,3,4,8],
         [1,9,8,3,4,2,5,6,7],
         [8,5,9,7,6,1,4,2,3],
         [4,2,6,8,5,3,7,9,1],
         [7,1,3,9,2,4,8,5,6],
         [9,6,1,5,3,7,2,8,4],
         [2,8,7,4,1,9,6,3,5],
         [3,4,5,2,8,6,1,7,9]]

# check_sudoku should return False
invalid = [[5,3,4,6,7,8,9,1,2],
           [6,7,2,1,9,5,3,4,8],
           [1,9,8,3,8,2,5,6,7],
           [8,5,9,7,6,1,4,2,3],
           [4,2,6,8,5,3,7,9,1],
           [7,1,3,9,2,4,8,5,6],
           [9,6,1,5,3,7,2,8,4],
           [2,8,7,4,1,9,6,3,5],
           [3,4,5,2,8,6,1,7,9]]

My code for the validator is the following:

def check_sudoku(grid):
    """ Gets a list of lists as input, and returns None for ill-formed
    Sudoku grids, False for invalid ones, and True for valid.
    """
    assert type(grid) == list

    # First, we need to make sure that the grid is 9x9.
    if len(grid) != 9:
        return None
    row_len = map(lambda x: len(x) == 9, grid)
    if False in row_len:
        return None

    # next we need to make sure that the numbers are between 0 and 9
    max_len = map(max, grid)
    min_len = map(min, grid)
    more_than_ten  = filter(lambda x: x > 9, max_len)
    less_than_zero = filter(lambda x: x < 0, min_len)
    if len(more_than_ten) or len(less_than_zero):
        return False

    # now we need to make sure that each number is present only one time in a row
    one_per_row = map(lambda x: len(set(x)) == 9, grid)
    if False in one_per_row:
        return False

    # and next up is making sure that each number is present only one time in a column
    # to do that, we want to follow the previous procedure, but for it to be valid, we
    # shall first need to turn each column into a line
    columns_to_lines = []
    for i in range(0, 9):
        column = []
        for z in range(0, 9):
            column.append(grid[i][z])
        columns_to_lines.append(column)
    one_per_column = map(lambda x: len(set(x)) == 9, columns_to_lines)
    if False in one_per_column:
        return False

    # for the last check, we need to ensure that each number is unique in the 3x3 grids
    # we can do this by creating a single 9 list element out of the 3x3 grid, and perform
    # the same procedure as above.
    three_times_three = []
    first_one   = []
    second_one  = []
    third_one   = []
    times_break = 0
    for i in range(0, 9):
        first_one.append(grid[i][0:3])
        second_one.append(grid[i][3:6])
        third_one.append(grid[i][6:9])
        times_break += 1
        if times_break > 2:
            three_times_three.append(first_one)
            three_times_three.append(second_one)
            three_times_three.append(third_one)
            first_one  = []
            second_one = []
            third_one  = []
            times_break = 0

    # collapse the lists because they now contain sublists, making it
    # slightly icky for the rest of the pipeline
    three_times_three = map(lambda l: sum(l, []), three_times_three)
    one_per_three_box = map(lambda x: len(set(x)) == 9, three_times_three)
    if False in one_per_three_box:
        return False

    return True

I wrote it all in functional style - that's what you get after a whole summer full of Scheme. My question is, is this considered high quality readable code? Would you consider it hard to read instead? If so why? What could you do better?

The solution is generally good.

The docstring is OK, but could be better if it more clearly stated the purpose of the function.  Follow the PEP 257 conventions for formatting.  Use the imperative rather than the indicative mood.

The comment blocks throughout the function are useful, but could be less verbose.  For example, you can cut out filler words like "First, we need to …".

Instead of assert type(grid) == list, I would assert isinstance(grid, list) to state what you really require without being too strict about the type, since duck-typing is the norm in Python.

You used this to validate the row lengths:

row_len = map(lambda x: len(x) == 9, grid)
if False in row_len:
    return None

Here, as in many places throughout the function, you used x as dummy variable.  x is vague, and has a connotation of being a floating-point number.  Try to pick more helpful names, as in lambda row: len(row) == 9.

Throughout this exercise, the all() built-in function and generator expressions are your friend.  These are ways to write compact Pythonic code that is in the spirit of functional programming without actually using map and lambda explicitly.

I don't think that this block is necessary:

# next we need to make sure that the numbers are between 0 and 9
max_len = map(max, grid)
min_len = map(min, grid)
more_than_ten  = filter(lambda x: x > 9, max_len)
less_than_zero = filter(lambda x: x < 0, min_len)
if len(more_than_ten) or len(less_than_zero):
    return False

Your strategy is to first ensure that the entire grid contains nothing but digits 1 to 9, then ensure that each set contains nine distinct members.  It would be simpler and clearer to eliminate this block, then check whether each set is equal to set(range(1, 10)).

For example, to check each row…

    DIGITS = set(range(1, 10))
    if not all(set(row) == DIGITS for row in grid):
        return False

Checking rows is easy; checking columns is slightly more challenging.  Transposing the grid can easily be accomplished with a list comprehension.

Checking each 3×3 square is more challenging.  You used one loop to build a bunch of lists of lists, then cleaned up the "ickiness" by flattening it.  You could avoid the flattening step by using .extend() instead of .append().

But really, what you should be using is itertools.product().

>>> from pprint import pprint as pp
>>> from itertools import product
>>> THREES = [(0, 1, 2), (3, 4, 5), (6, 7, 8)]
>>> pp(list(product(THREES, THREES)))
[((0, 1, 2), (0, 1, 2)),
 ((0, 1, 2), (3, 4, 5)),
 ((0, 1, 2), (6, 7, 8)),
 ((3, 4, 5), (0, 1, 2)),
 ((3, 4, 5), (3, 4, 5)),
 ((3, 4, 5), (6, 7, 8)),
 ((6, 7, 8), (0, 1, 2)),
 ((6, 7, 8), (3, 4, 5)),
 ((6, 7, 8), (6, 7, 8))]
>>> pp([
...     [(r, c) for r, c in product(row_block, col_block)]
...     for row_block, col_block in product(THREES, THREES)
... ])
[[(0, 0), (0, 1), (0, 2), (1, 0), (1, 1), (1, 2), (2, 0), (2, 1), (2, 2)],
 [(0, 3), (0, 4), (0, 5), (1, 3), (1, 4), (1, 5), (2, 3), (2, 4), (2, 5)],
 [(0, 6), (0, 7), (0, 8), (1, 6), (1, 7), (1, 8), (2, 6), (2, 7), (2, 8)],
 [(3, 0), (3, 1), (3, 2), (4, 0), (4, 1), (4, 2), (5, 0), (5, 1), (5, 2)],
 [(3, 3), (3, 4), (3, 5), (4, 3), (4, 4), (4, 5), (5, 3), (5, 4), (5, 5)],
 [(3, 6), (3, 7), (3, 8), (4, 6), (4, 7), (4, 8), (5, 6), (5, 7), (5, 8)],
 [(6, 0), (6, 1), (6, 2), (7, 0), (7, 1), (7, 2), (8, 0), (8, 1), (8, 2)],
 [(6, 3), (6, 4), (6, 5), (7, 3), (7, 4), (7, 5), (8, 3), (8, 4), (8, 5)],
 [(6, 6), (6, 7), (6, 8), (7, 6), (7, 7), (7, 8), (8, 6), (8, 7), (8, 8)]]

I could go further and validate all nine 3×3 blocks using just one statement, but that could be a bit too overwhelming.

Suggested solution

from itertools import product

def check_sudoku(grid):
    """Validate a sudoku solution.

    Given a grid as a list of lists, return None if it is ill-formed,
    False if it is invalid, or True if it is a valid solution.
    """
    assert isinstance(grid, list)

    # Check that the grid is 9x9.
    if len(grid) != 9 or not all(len(row) == 9 for row in grid):
        return None

    DIGITS = set(range(1, 10))

    # Check that each number appears exactly once per row
    if not all(set(row) == DIGITS for row in grid):
        return False

    # Check that each number appears exactly once per column
    columns = [[row[c] for row in grid] for c in range(9)]
    if not all(set(col) == DIGITS for col in columns):
        return False

    # Check that each number appears exactly once per 3x3 grid
    THREES = [(0, 1, 2), (3, 4, 5), (6, 7, 8)]
    for row_block, col_block in product(THREES, THREES):
        block = [grid[r][c] for r, c in product(row_block, col_block)]
        if set(block) != DIGITS:
            return False

    return True

Functional solution

Since you mentioned an interest in functional programming, I thought I should also present a more functional variant for your consideration.

from itertools import product

def check_sudoku(grid):
    """Validate a sudoku solution.

    Given a grid as a list of lists, return None if it is ill-formed,
    False if it is invalid, or True if it is a valid solution.
    """
    assert isinstance(grid, list)

    # Check that the grid is 9x9.
    if len(grid) != 9 or not all(len(row) == 9 for row in grid):
        return None

    DIGITS = set(range(1, 10))
    THREES = [(0, 1, 2), (3, 4, 5), (6, 7, 8)]

    def correct(groups):
        return all(set(group) == DIGITS for group in groups)

    rows = grid
    columns = zip(*grid)
    squares3x3 = [
        [grid[r][c] for r, c in product(row_block, col_block)]
        for row_block, col_block in product(THREES, THREES)
    ]

    return correct(rows) and correct(columns) and correct(squares3x3)


"back substitution" method for solving linear system I'm using numpy to write the "back substitution" method for solving linear system where "A" is a nonsingular upper triangular matrix.

import numpy as np

def upperTriSol(A, b):
    n = np.size(b)
    x = np.zeros_like(b)

    x[-1] = 1. / A[-1, -1] * b[-1]
    for i in xrange(n-2, -1, -1):
        x[i] = 1. / A[i, i] * (b[i] - np.sum(A[i, i+1:] * x[i+1:]))

    return x

I know that "for loops" are slow, so I wonder if there is any way to avoid them in this case? If not, is there a "correct" way to write efficient "for loops" in numpy?

A few quick suggestions that I may flesh out in further edits:

If looks like x[i] is a function of x[i+1] and selected items from A and b.  Then it could also be seen as forward itertion on a reversed list, xr[i+1] dependent on xr[i].

np.cumsum and related accumulated functions are often useful in problems like this.

There's a set of tri functions that give you indices and values on upper and lower triangular matrices.

Typically 'vectorizing' in numpy requires stepping back a bit from the problem and imagining in terms of operations on a table or 2d array of values.

(this question would have gotten more attention on SO).


Making a .bat batch command-line interface Recently I am working on batch to make a very simple command line interface.

The code looks like this:

@echo off
echo Simple command-line interface
echo.
:input.get
set /p input="COMMAND\>"
set input5=%input:~0,5%
if "%input%"=="something" goto something
if "%input5%"=="echo " goto echo

:echo
set content=%input:~5%
echo.%content%
goto input.get

:something
::do something
goto input.get

Is this really a good way to do it? I have posted a question about "How to check whether labels exist" in stack overflow, but if i input "echo hi" then it will be nearly impossible to check the tag.

Is it good to use both ways? (check whether label exist -> if not exist -> do "if" check)

You can use a FOR /F to parse out the command from the arguments.

I would define a variable containing a delimited list of all valid commands. Then you can use simple search and replace to validate whether the user entered command is valid.

Addition of a new command is as easy as adding the command to the list, and creating a labeled subroutine for the new command.

I would use CALL instead of GOTO so that each routine can easily parse the arguments. The only disadvantage is CALL will double up quoted carets ("^" becomes "^^")

Here is a basic framework that can easily be extended. Note that user entered ! will be corrupted (or expanded) because of delayed expansion. There are simple ways to get around this limitation with additional code.

@echo off
setlocal enableDelayedExpansion
echo Simple command-line interface
echo(

:: Define all valid commands: make sure there is a space between each command
:: and also one at beginning and end
set "commands= something echo exit "

:input.get

:: Clear the existing value in case user hits <Enter> without entering anything
set "input="

:: Get the next command
set /p "input=COMMAND\>"

:: Parse command into command and arguments.
for /f "tokens=1* delims= " %%A in ("!input!") do (

  REM check if command is valid (not case sensitive) and act accordingly
  if "!commands: %%A =!" equ "!commands!" (
    echo Invalid command: %%A
  ) else if /i %%A equ exit (
    exit /b
  ) else (
    call :%%A %%B
  )
)
echo(
goto input.get

:something
echo Doing something with Arg1=[%1] and Arg2=[%2]
exit /b

:echo
echo(%*
exit /b