Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#26779 closed enhancement (fixed)

py3: fix graph_input.py and hypergraph_generators.py

Reported by: David Coudert Owned by:
Priority: major Milestone: sage-8.5
Component: graph theory Keywords: py3, graph
Cc: Travis Scrimshaw, Frédéric Chapoton, Erik Bray Merged in:
Authors: David Coudert Reviewers: Frédéric Chapoton
Report Upstream: N/A Work issues:
Branch: 016fa31 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description

doctest errors in hypergraph_generators.py are due to bytes vs str. We add a test in graph_input.py to fix the issue.

Change History (14)

comment:1 Changed 4 years ago by David Coudert

Branch: public/26779_graph_input
Cc: Travis Scrimshaw Frédéric Chapoton added
Commit: c58ed7920a65bfde0e651f5393ca165db546bccf
Status: newneeds_review

I don't know if it's the best way to do it.


New commits:

c58ed79trac #26779: fix hypergraph generators and graph input

comment:2 Changed 4 years ago by Frédéric Chapoton

Cc: Erik Bray added

probably, it would be better to use "bytes_to_str"

from sage.cpython.string import bytes_to_str

cc-ing Erik, who knows better about the unicode problem

comment:3 Changed 4 years ago by David Coudert

If it's safer I can use it. I was not aware of that method. Let me know if I should do the change.

comment:4 Changed 4 years ago by Frédéric Chapoton

yes, please use "bytes_to_str".

comment:5 Changed 4 years ago by git

Commit: c58ed7920a65bfde0e651f5393ca165db546bccf99e5b330baf3b8a5c7b16107ced5381cba83a2ea

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

5f3b84dtrac #26779: Merged with 8.5.beta6
99e5b33trac #26779: use bytes_to_str

comment:6 Changed 4 years ago by David Coudert

Thank you. I agree it is much cleaner this way.

Last edited 4 years ago by David Coudert (previous) (diff)

comment:7 Changed 4 years ago by Frédéric Chapoton

Reviewers: Frédéric Chapoton
Status: needs_reviewpositive_review

ok, let it be

comment:8 Changed 4 years ago by Erik Bray

Status: positive_reviewneeds_work

Please try to avoid using inline imports if it isn't strictly necessary; see recent discussion about this at https://trac.sagemath.org/ticket/26758#comment:11

comment:9 Changed 4 years ago by git

Commit: 99e5b330baf3b8a5c7b16107ced5381cba83a2ea016fa31eb0083267eaccf15c5a508da5aaabbbb1

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

016fa31trac #26779: move import to module level

comment:10 Changed 4 years ago by David Coudert

Status: needs_workneeds_review

I have not touched to existing imports. But it could also be done here if needed.

comment:11 Changed 4 years ago by Frédéric Chapoton

Status: needs_reviewpositive_review

ok

comment:12 Changed 4 years ago by Volker Braun

Branch: public/26779_graph_input016fa31eb0083267eaccf15c5a508da5aaabbbb1
Resolution: fixed
Status: positive_reviewclosed

comment:13 Changed 4 years ago by Erik Bray

Commit: 016fa31eb0083267eaccf15c5a508da5aaabbbb1

It would also be great to squash trivial changes like those together, although at least in this case both versions are at least valid (it's more of a problem when one commit contains trivial errors).

comment:14 Changed 4 years ago by Erik Bray

Didn't mean to delete that. I think that's a bug in the trac plugin...

Note: See TracTickets for help on using tickets.