Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[READY] Merge brombo/galgebra#15 #2

Merged
merged 36 commits into from Sep 21, 2018
Merged

[READY] Merge brombo/galgebra#15 #2

merged 36 commits into from Sep 21, 2018

Conversation

@utensil
Copy link
Member

utensil commented Sep 19, 2018

  • merge brombo/galgebra#15 based on pygae:master and resolve conflicts literally
  • setup CI preliminarily
  • Fixes tests under Python 2
  • Fixes tests under Python 3
@utensil

This comment has been minimized.

Copy link
Member Author

utensil commented Sep 19, 2018

Now there're 7 fails, 13 passes, because my PR has fixed all 10 tests and merging literally has kept 10 tests with 7 fails and 3 passes from the master branch, will check and clean up later.

@utensil

This comment has been minimized.

Copy link
Member Author

utensil commented Sep 19, 2018

image

@hugohadfield @arsenovic I don't have the permission to set up a CI for pygae/galgebra, can you help me with the permission? Thanks.

@utensil utensil force-pushed the utensil-contrib:15-pip branch from 34f6577 to db89f83 Sep 19, 2018
@hugohadfield

This comment has been minimized.

Copy link
Collaborator

hugohadfield commented Sep 19, 2018

@utensil sorry I was asleep! Didn't realise I had set your permission settings wrong in the invite, you are now an owner in pygae, so hopefully you should be able to do everything you want without permission problems!

@utensil utensil force-pushed the utensil-contrib:15-pip branch 2 times, most recently from b7a0500 to 01c9d7b Sep 19, 2018
@arsenovic

This comment has been minimized.

Copy link
Member

arsenovic commented Sep 19, 2018

i gave the galgebra group to admin rights for the galgebra repo,
its under repo settings-> collaborators and team

@utensil utensil force-pushed the utensil-contrib:15-pip branch from 3409b68 to d4c5efd Sep 20, 2018
@utensil utensil force-pushed the utensil-contrib:15-pip branch from c805574 to b4a3672 Sep 20, 2018
@utensil utensil changed the title [DON't MERGE YET] Merge brombo/galgebra#15 [READY] Merge brombo/galgebra#15 Sep 20, 2018
@utensil utensil requested a review from hugohadfield Sep 20, 2018
@utensil utensil self-assigned this Sep 20, 2018
@utensil

This comment has been minimized.

Copy link
Member Author

utensil commented Sep 20, 2018

@arsenovic @hugohadfield This PR has completed all 4 tasks and passes 12 tests under both py 2 & 3, please review, comments welcome. 😄

@utensil utensil requested a review from arsenovic Sep 20, 2018
Copy link
Collaborator

hugohadfield left a comment

Looks good :) Nice job Utensil!

Copy link
Member

arsenovic left a comment

i just glossed over these changes but they look good. glad to see the project start moving again.

global n,nbar
Fx = HALF*((x*x)*n+2*x-nbar)
global n, nbar
Fx = ((x * x) * n + 2 * x - nbar) / 2

This comment has been minimized.

Copy link
@utensil

utensil Sep 21, 2018

Author Member

Some doubts about the division here, will look into it later.

This comment has been minimized.

Copy link
@utensil

utensil Sep 21, 2018

Author Member

This was originally for a rational division. Better keep the HALF for now.

return self

def sort_terms(self):
self.terms.sort(key=operator.itemgetter(1), cmp=Pdop.compare)
# self.terms.sort(key=operator.itemgetter(1))
self.terms.sort(key=cmp_to_key(Pdop.compare))

This comment has been minimized.

Copy link
@utensil

utensil Sep 21, 2018

Author Member

It was using both key and cmp, and the cmp obviously contains more logics which I chose to keep instead of key.

setup.py Outdated
sys.argv.remove('--noconflict')
except Exception as e:
pass
ga_namespace = 'galgebra' if noconflict else ''

This comment has been minimized.

Copy link
@utensil

utensil Sep 21, 2018

Author Member

These are probably obsolete.

@utensil

This comment has been minimized.

Copy link
Member Author

utensil commented Sep 21, 2018

Thanks, then I'll merge after some small revision 😃

Here's a little change log besides changes in brombo/galgebra#15 :

  • .travis.yml: runs test_test.py and test_mv.py for py 2.7 & 3.6 using py.test, no coverage yet, installation is done by python setup.py install instead of setting pth
  • change the way of importing modules to make it compatible with both py 2 & 3, circular dependencies removed (except a few helper functions depending on them which can be at a better place or removed later)
    • use from . import in modules
    • use from galgebra.ga import-ish in tests
    • remove self-dependencies and the circular dependencies between mv and ga
  • fix illegal escape sequence warning by adding r prefix to strings with LaTeX in it
  • fix most of py3 compatiblity issues by 2to3 and __future__ imports
  • manually fix the following py3 compatibility issues:
    • make string type checking work under both 2 and 3 without importing unicode_literals from __future__
    • add a naive compiler.ast. flatten implementation to make it work under 3 (it's gone since 3)
    • fix Sdop.sort_terms by using key function with the help of functools.cmp_to_key (cmp is gone since 3)
    • fix bugs in _print_Derivative implementation in GaPrinter when handling the variables and the powers of them
  • .gitignore: add a few more file types
@utensil utensil merged commit 7c664a1 into pygae:master Sep 21, 2018
2 checks passed
2 checks passed
Travis CI - Pull Request Build Passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@utensil utensil mentioned this pull request Sep 21, 2018
6 of 19 tasks complete
@utensil utensil mentioned this pull request Feb 14, 2019
5 of 5 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.