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

Jump to bottom

Py3 : database.oeis implement __getitem__ to replace __getslice__ #26704

Closed
vinklein mannequin opened this issue Nov 14, 2018 · 27 comments
Closed

Py3 : database.oeis implement __getitem__ to replace __getslice__ #26704

vinklein mannequin opened this issue Nov 14, 2018 · 27 comments
Labels
c: python3 p: major / 3 t: enhancement
Milestone
sage-8.5

Comments

@vinklein
Copy link
Mannequin

vinklein mannequin commented Nov 14, 2018

Fix database.oeis module for python3 :

  • Remove a useless test
  • Define __getitem__ method with for FancyTuple object. It replace __getslice__ method in python3.
    As __getslice__ is still defined in tuple in python 2.7 we still need a __getslice__ method in FancyTuple to override it.

__getslice__ is deprecated since python 2.6 and removed in python 3.x

Component: python3

Keywords: thursdaysbdx

Author: Vincent Klein

Branch/Commit: 7d03e29

Reviewer: Frédéric Chapoton

Issue created by migration from https://trac.sagemath.org/ticket/26704
@vinklein vinklein mannequin added this to the sage-8.5 milestone Nov 14, 2018
@vinklein vinklein mannequin added c: python3 labels Nov 14, 2018
@vinklein

This comment has been minimized.

Sign in to view
@vinklein
Copy link
Mannequin Author

vinklein mannequin commented Nov 14, 2018

Branch: u/vklein/develop

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 14, 2018

Commit: ad8e0b8

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 14, 2018

Branch pushed to git repo; I updated commit sha1. New commits:

ad8e0b8Trac #26704: Define `__getslice__` for python2

@vinklein
Copy link
Mannequin Author

vinklein mannequin commented Nov 14, 2018

Changed commit from ad8e0b8 to none

@vinklein
Copy link
Mannequin Author

vinklein mannequin commented Nov 14, 2018

Changed branch from u/vklein/develop to none

@vinklein

This comment has been minimized.

Sign in to view
@vinklein vinklein mannequin changed the title Py3 : database.oeis replace __getslice__ by __getitem__ Py3 : database.oeis implement __getitem__ to replace __getslice__ Nov 14, 2018
@vinklein
Copy link
Mannequin Author

vinklein mannequin commented Nov 14, 2018

Branch: u/vklein/26704

@vinklein
Copy link
Mannequin Author

vinklein mannequin commented Nov 14, 2018

Commit: ad8e0b8

@vinklein
Copy link
Mannequin Author

vinklein mannequin commented Nov 14, 2018

New commits:

840f17aTrac #26704: Fix database.oeis module for python3 :
ad8e0b8Trac #26704: Define `__getslice__` for python2

@vinklein vinklein mannequin added the s: needs review label Nov 14, 2018
@fchapoton
Copy link
Contributor

comment:7

Please add documentation and doctests to the new __getitem__ method.

@vinklein
Copy link
Mannequin Author

vinklein mannequin commented Nov 22, 2018

comment:8

It would be the same doctests as those of __getslice__.

What do you think is the most appropriate ? :

  1. Leave as it is.
  2. Add documentation.
  3. Add documentation and doctests and then have the sames tests two times.

@fchapoton
Copy link
Contributor

comment:9

either duplicate the doc or add a doctest for when the argument is not a slice but just one element

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 22, 2018

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

e3499b4Trac #26704: Fix database.oeis module for python3 :
9cae5bbTrac #26704: Define `__getslice__` for python2
34c80dcTrac #26704: Add doctests for __getitem__.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 22, 2018

Changed commit from ad8e0b8 to 34c80dc

@vinklein
Copy link
Mannequin Author

vinklein mannequin commented Nov 22, 2018

comment:11

Replying to @fchapoton:

either duplicate the doc or add a doctest for when the argument is not a slice but just one element

I ve done the second one.

@fchapoton
Copy link
Contributor

comment:12

manque une ligne vide en dessous de TESTS::

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 22, 2018

Changed commit from 34c80dc to af1e030

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 22, 2018

Branch pushed to git repo; I updated commit sha1. New commits:

af1e030Trac #26704: Add a mising Blankline after TESTS::.

@vinklein
Copy link
Mannequin Author

vinklein mannequin commented Nov 22, 2018

comment:14

Replying to @fchapoton:

manque une ligne vide en dessous de TESTS::

My bad.

And my newest tests won't work in py2 as sage: ft[0] return '\xc3\xa9'.

@vinklein vinklein mannequin added s: needs work and removed s: needs review labels Nov 22, 2018
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 22, 2018

Changed commit from af1e030 to 7d03e29

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 22, 2018

Branch pushed to git repo; I updated commit sha1. New commits:

7d03e29Trac #26704: Fix `__getitem__` doctests for python2

@vinklein
Copy link
Mannequin Author

vinklein mannequin commented Nov 22, 2018

Changed keywords from none to thursdaysbdx

@vinklein
Copy link
Mannequin Author

vinklein mannequin commented Nov 22, 2018

comment:17

Fixed

@fchapoton
Copy link
Contributor

Reviewer: Frédéric Chapoton

@fchapoton
Copy link
Contributor

comment:19

ok, thanks

@vbraun
Copy link
Member

vbraun commented Nov 23, 2018

Changed branch from u/vklein/26704 to 7d03e29

@vbraun vbraun closed this as completed in 7d7d7ea Nov 23, 2018
@fchapoton fchapoton mentioned this issue Jan 1, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c: python3 p: major / 3 t: enhancement
Projects
None yet
Milestone
sage-8.5
Development

No branches or pull requests
2 participants
@vbraun @fchapoton