Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Now there aren't bare except: and it is explicit what exception is thrown #784

Closed
wants to merge 11 commits into
from

Conversation

Projects
None yet
6 participants
Member

smichr commented Nov 29, 2011

This needs to be rebased. Also, I think we want to leave mpmath alone as that is something we include but don't maintain with sympy.

I don't know what rebase means. And as to leaving mpmath alone is that means that i shouldn't change the statements "except:" ?

Contributor

vperic commented Nov 29, 2011

Yeah, I don't think we should make any change to mpmath. And yes, you should probably rebase it as it no longer applies cleanly on master (though it's probably something trivial). We have a nicely written guide for rebasing here:

https://github.com/sympy/sympy/wiki/Development-workflow#wiki-rebasing

If you are having any sort of problems, please don't hesitate to ask.

Anyway, I looked at this and I don't see anything wrong, but lets wait for someone else to +1 it too.

I have some problem with committing. I changed the mpmath files. And followed the instruction on rebasing from the link. I have done $ git add sympy/mpmath
$ git commit -m "Rebased"
$ git push -f https://tsmars15@github.com/tsmars15/sympy.git remove_bare_except_statments
then got the msg:
Everything up-to-date
$ git remote -v
github git@ithub.com:tsmar15/sympy.git (fetch)
github git@ithub.com:tsmar15/sympy.git (push)
origin git://github.com/sympy/sympy.git (fetch)
origin git://github.com/sympy/sympy.git (push)
$ git checkout master
and i got this error msg:
error: Your local changes to the following files would be overwritten by checkout:
sympy/plotting/color_scheme.py
Please, commit your changes or stash them before you can switch branches.
Aborting
and also when i write git commit --interactive it doesn't say that i have changed mpmath files despite the fact that i have changed them.

I can't make rebase. I tried so many times and i got this error
Cannot rebase: You have unstaged changes.
Additionally, your index contains uncommitted changes.
Please commit or stash them.

If it is OK can i make new branch where mpmath is not changed?

Contributor

goodok commented Nov 30, 2011

  1. It is not nessesary, I think: '$ git add sympy/mpmath'

  2. What is the output of this command?

    $ git status

  3. Note that git commit -m "Rebased" commits only files that you manually added with git add command, Use -a option: git commit -a -m "Rebased" to commit all files automatically. But check git status before it.

  4. The letter g is omittet in remotes github git@ithub.com:tsmar15/sympy.git (fetch). So:

    $ git remote rm github
    $ git remote add github git@github.com:tsmar15/sympy.git

  5. It is not necessary, but more convenient to use:

    $ git push -f github remove_bare_except_statments

instead of

git push -f https://tsmars15@github.com/tsmars15/sympy.git remove_bare_except_statments

Taking into account, that remote github is defined correctly (see §3)

Contributor

goodok commented Nov 30, 2011

(see §4)

Owner

asmeurer commented Nov 30, 2011

Yes, you can make a new branch if you want.

Regarding rebasing, it's because you have changes that you haven't committed (i.e., that you haven't pushed up to here) that it isn't working. You can see these with the commands git diff and git diff --cached.

You can quickly undo these by doing

git stash
git rebase master

And then, when you are done,

git stash pop

When you are rebasing, and otherwise, follow the onscreen instructions. They will help you a lot.

Contributor

goodok commented Nov 30, 2011

If it is OK can i make new branch where mpmath is not changed?

Yes, but before you must commit all chacnges in current branch (see my previous comment).

If you fails with it then you can leave your current branch with the help of git stash command (which is aimed for temporary express switching):

    $ git stash
    $ git checkout master 

Do not forget to refreash master branch (git pull), and create new one from master:
$ git checkout master
$ git pull
$ git checkout -b remove_bare_except_statments_2

After you change code to implement your fitcher, then you can play with renaming of branches, (so you can connect it to this pull request).

Thank you for your answers. I will make new branch since i really messed up with this one :)

I changed mpmath to its old state by editing from here. I hope that this will be good for you to merge it. I had some problems with the folders on my computer. Since my timezone is UTC + 2 i couldn't wait for your comments and last night before you response i tried to rebase then when i couldn't do it i tried to make a new branch.... The result was one big mess. I really hope that these edited files will be enough. I am really really sorry that i couldn't rebase my commit and i fail at creating and committing new branch. I hope you approve my commit since this is my first Google Code in task and i just want to do it well and to be accepted. Thank you for trying to help me. :)

Member

smichr commented Nov 30, 2011

I rebased this and pushed to my branch named bare if that will help. There was a slight problem with the runtests file; you might want to double check that one. There were only two doctest failures:

_________ sympy.physics.mechanics.essential.ReferenceFrame.orientnew __________
File "C:\Users\leslie\sympy\sympy\physics\mechanics\essential.py", line 1002, in
 sympy.physics.mechanics.essential.ReferenceFrame.orientnew
Failed example:
    B.orient(N, 'Body', [0, 0, 0], 'XYX')
Exception raised:
    Traceback (most recent call last):
      File "C:\Python27\lib\doctest.py", line 1254, in __run
        compileflags, 1) in test.globs
      File "<doctest sympy.physics.mechanics.essential.ReferenceFrame.orientnew[
12]>", line 1, in <module>
        B.orient(N, 'Body', [0, 0, 0], 'XYX')
      File "C:\Users\leslie\sympy\sympy\physics\mechanics\essential.py", line 92
5, in orient
        td = solve(templist, [u1, u2, u3])
      File "C:\Users\leslie\sympy\sympy\solvers\solvers.py", line 617, in solve
        solution = _solve_system(f, symbols, **flags)
      File "C:\Users\leslie\sympy\sympy\solvers\solvers.py", line 1006, in _solv
e_system
        poly = g.as_poly(*symbols, **{'extension': True})
      File "C:\Users\leslie\sympy\sympy\core\basic.py", line 659, in as_poly
        poly = Poly(self, *gens, **args)
      File "C:\Users\leslie\sympy\sympy\polys\polytools.py", line 96, in __new__

        return cls._from_expr(rep, opt)
      File "C:\Users\leslie\sympy\sympy\polys\polytools.py", line 204, in _from_
expr
        return cls._from_dict(rep, opt)
      File "C:\Users\leslie\sympy\sympy\polys\polytools.py", line 149, in _from_
dict
        domain, rep = construct_domain(rep, opt=opt)
      File "C:\Users\leslie\sympy\sympy\polys\constructor.py", line 208, in cons
truct_domain
        result = _construct_simple(coeffs, opt)
      File "C:\Users\leslie\sympy\sympy\polys\constructor.py", line 53, in _cons
truct_simple
        result.append(domain.from_sympy(coeff))
      File "C:\Users\leslie\sympy\sympy\polys\domains\pythonrationalfield.py", l
ine 34, in from_sympy
        raise CoercionFailed("expected `Rational` object, got %s" % a)
    CoercionFailed: expected `Rational` object, got oo
_______________________________________________________________________________
___________ sympy.physics.mechanics.essential.ReferenceFrame.orient ___________
File "C:\Users\leslie\sympy\sympy\physics\mechanics\essential.py", line 798, in
sympy.physics.mechanics.essential.ReferenceFrame.orient
Failed example:
    B.orient(N, 'Body', [0, 0, 0], 'XYX')
Exception raised:
    Traceback (most recent call last):
      File "C:\Python27\lib\doctest.py", line 1254, in __run
        compileflags, 1) in test.globs
      File "<doctest sympy.physics.mechanics.essential.ReferenceFrame.orient[7]>
", line 1, in <module>
        B.orient(N, 'Body', [0, 0, 0], 'XYX')
      File "C:\Users\leslie\sympy\sympy\physics\mechanics\essential.py", line 92
5, in orient
        td = solve(templist, [u1, u2, u3])
      File "C:\Users\leslie\sympy\sympy\solvers\solvers.py", line 617, in solve
        solution = _solve_system(f, symbols, **flags)
      File "C:\Users\leslie\sympy\sympy\solvers\solvers.py", line 1006, in _solv
e_system
        poly = g.as_poly(*symbols, **{'extension': True})
      File "C:\Users\leslie\sympy\sympy\core\basic.py", line 659, in as_poly
        poly = Poly(self, *gens, **args)
      File "C:\Users\leslie\sympy\sympy\polys\polytools.py", line 96, in __new__

        return cls._from_expr(rep, opt)
      File "C:\Users\leslie\sympy\sympy\polys\polytools.py", line 204, in _from_
expr
        return cls._from_dict(rep, opt)
      File "C:\Users\leslie\sympy\sympy\polys\polytools.py", line 149, in _from_
dict
        domain, rep = construct_domain(rep, opt=opt)
      File "C:\Users\leslie\sympy\sympy\polys\constructor.py", line 208, in cons
truct_domain
        result = _construct_simple(coeffs, opt)
      File "C:\Users\leslie\sympy\sympy\polys\constructor.py", line 53, in _cons
truct_simple
        result.append(domain.from_sympy(coeff))
      File "C:\Users\leslie\sympy\sympy\polys\domains\pythonrationalfield.py", l
ine 34, in from_sympy
        raise CoercionFailed("expected `Rational` object, got %s" % a)
    CoercionFailed: expected `Rational` object, got oo

=========== tests finished: 1419 passed, 2 failed, in 46.95 seconds ===========
Contributor

goodok commented Nov 30, 2011

Doctests fails for B.orient(N, 'Body', [0, 0, 0], 'XYX') cause physics/mechanics/essential.py#L930.

When the exception catcher was bare, then it catch the:

File: ... physics/mechanics/essential.py", line 934, in orient
        self._ang_vel_dict.update({parent: wvec})
UnboundLocalError: local variable 'wvec' referenced before assignment

indeed. (It is from recurrsion)

OK,thanks i will fix that.I forgot to run doctest before uploading.Sorry.

Contributor

goodok commented Nov 30, 2011

@tsmars15 Note that in my previous comment there is a mistake about UnboundLocalError. It is CoercionFailed indeed.

This code of .orient() calls "solve" at essential.py#L925 to solve the equations. And "solve" fails with this exception (CoercionFailed)

I wrote UnboundLocalError but the test still fails. Since it is from recursion i wrote OverflowError too but still doesn't pass.

Contributor

goodok commented Nov 30, 2011

Sorry, try CoercionFailed which can be imported from sympy.polys.polyerrors.

Thanks @goodok it worked! I am running all tests again(just for case) :) and then i will edit it here.

When i runned the test i got this error:
____________ sympy/utilities/tests/test_code_quality.py:test_files _____________
File "/home/mars15/Desktop/branch3/sympy/utilities/tests/test_code_quality.py", line 121, in test_files
check_directory_tree(SYMPY_PATH, test, exclude)
File "/home/mars15/Desktop/branch3/sympy/utilities/tests/test_code_quality.py", line 68, in check_directory_tree
file_check(fname)
File "/home/mars15/Desktop/branch3/sympy/utilities/tests/test_code_quality.py", line 91, in test
assert False, message_tabs % (fname, idx+1)
AssertionError: File contains tabs instead of spaces: /home/mars15/Desktop/branch3/sympy/physics/mechanics/essential.py, line 919.

Then i used space instead of tabs when i wrote that:
from sympy.polys.polyerrors import CoercionFailed
and the test passed

In the last upload i removed FlagError since it was useless and put space after the comma between CoercionFailed and AssertionError.

Member

Krastanov commented Nov 30, 2011

This can not be merged at the moment. You will need to rebase again.

What should i change? And i can't rebase since i have deleted the folder in which was remove_bare_except_statments branch on my computer. And as i said i have problem with creating new branch. Can i do something else in order the branch to be accepted?

Contributor

goodok commented Dec 1, 2011

And as i said i have problem with creating new branch. Can i do something else in order the branch to be accepted?

I looked at the https://github.com/sympy/sympy/pull/784/files .
And I think that the easiest way is to save this page (as a all you files) and begin from the start: follow the page [[GCI-2011-Landing]]
and begin from the section "Development Workflow", subsection 1, part (d). That is clone the clean SymPy, and create you branch again, and again apply the changes which you have done.

Member

smichr commented Dec 1, 2011

The only rebase problem was in runtests. I've rebased this and am
running tests. If they all pass I'll commit this unless you want to go
through the exercise of following Alexey's advice and trying again.
It's a good exercise. Let me know what you would like to do.

If the tests don't pass i will follow the advice. But if they do then i have done the task,right?

@smichr smichr closed this Dec 1, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment