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

Support 3.8 with tests fix #263

Closed
wants to merge 12 commits into from
Closed

Support 3.8 with tests fix #263

wants to merge 12 commits into from

Conversation

@uburuntu
Copy link
Contributor

@uburuntu uburuntu commented May 26, 2020

Hi!

I want to use Fire in project with Python 3.8, so I think library should be officially supported for 3.8.

I added travis run for 3.8 and one test testInitRequiresFlagSyntaxSubclassNamedTuple got failed: link.

Investigation showed that now namedtuple attributes of type collections._tuplegetter since 3.8, not property: link, commit:

So I added backward compatibility check in MemberVisible function.

One not solved trouble: pytype does not support 3.8 (google/pytype#587) -- https://travis-ci.org/github/uburuntu/python-fire, but I can add check in travis.yml to skip it currently.

@googlebot googlebot added the cla: yes label May 26, 2020
@dbieber
Copy link
Member

@dbieber dbieber commented May 26, 2020

Thanks 🙏
Skipping pytype for 3.8 is fine.

@uburuntu uburuntu force-pushed the uburuntu:master branch from 9cc1a73 to ef2d7a9 May 26, 2020
@uburuntu
Copy link
Contributor Author

@uburuntu uburuntu commented May 26, 2020

uburuntu added 2 commits Jun 4, 2020
@uburuntu
Copy link
Contributor Author

@uburuntu uburuntu commented Jun 4, 2020

Now with pytype 🙃

Build status: https://travis-ci.org/github/uburuntu/python-fire

@ssbarnea
Copy link

@ssbarnea ssbarnea commented Jun 7, 2020

This is going to show us how fast the project is going to adapt. Based on #139 which is almost two years old, I would say that a lot. I am keeping an eye on Fire but I will avoid it for the moment on my own projects.

@dbieber
Copy link
Member

@dbieber dbieber commented Jun 8, 2020

Thanks for the update @uburuntu and for the review @ssbarnea.
This LGTM.

Merge will happen within the next 2-3 weeks, I expect.

python-fire-bot pushed a commit that referenced this pull request Jun 12, 2020
- Enables Travis for 3.8
- Fixes namedtuple test for 3.8

COPYBARA_INTEGRATE_REVIEW=#263 from uburuntu:master efaf65f
PiperOrigin-RevId: 316134191
Change-Id: I5e3ef20b488fa917b81525d5936a8e147b0458e1
@dbieber
Copy link
Member

@dbieber dbieber commented Jun 12, 2020

Merged in 457b156

@dbieber dbieber closed this Jun 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.