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

Supporting Python <3.6 #4

Closed
cristgal opened this issue May 11, 2018 · 6 comments
Closed

Supporting Python <3.6 #4

cristgal opened this issue May 11, 2018 · 6 comments
Labels
platform Everything related to the Operating System or the Python runtime Pyre is running on

Comments

@cristgal
Copy link

[root@c7-linpe-build ~]# pip3 install pyre-check
Collecting pyre-check
Downloading https://files.pythonhosted.org/packages/e4/96/72a7048e5f340678c6eb48163b7bdc49926cdac2dff8bb5d935563e50b7f/pyre_check-0.0.5-py3-none-manylinux1_x86_64.whl (7.7MB)
100% |████████████████████████████████| 7.7MB 3.5MB/s
Installing collected packages: pyre-check
Successfully installed pyre-check-0.0.5

[root@c7-linpe-build ~]# pyre
Traceback (most recent call last):
File "/usr/bin/pyre", line 7, in
from pyre_check.pyre import main
File "/usr/lib/python3.4/site-packages/pyre_check/init.py", line 151
**normals,
^
SyntaxError: invalid syntax

[root@c7-linpe-build ~]# rpm -qi python34
Name : python34
Version : 3.4.8
Release : 1.el7

[root@c7-linpe-build ~]# cat /etc/redhat-release
CentOS Linux release 7.5.1804 (Core)

@ambv
Copy link

ambv commented May 11, 2018

Currently pyre_check is Python 3.6+ only but we should be able to make the runner work with 3.4+.

@dark
Copy link
Contributor

dark commented May 11, 2018

@cristgal Thanks for the report. As @ambv mentioned, we use 3.6 for our development.
We will discuss with the team to see if it's feasible to support older release trains.

@dark dark added the platform Everything related to the Operating System or the Python runtime Pyre is running on label May 12, 2018
@dark dark changed the title error on centos 7.4 & 7.5 Supporting Python <3.6 May 12, 2018
@dark
Copy link
Contributor

dark commented May 12, 2018

Renamed for clarity.

@dkgi
Copy link
Contributor

dkgi commented May 13, 2018

Fixed this for 3.5 with #32. Unfortunately I was not able to get a python 3.4 working on my system.

facebook-github-bot pushed a commit that referenced this issue May 13, 2018
Summary:
Issue #4 was raised to support older Python versions. PR #32
introduced support for Python 3.5. We might not be able to support
older versions, and we shouldn't be installable on them if we don't
support them.

Reviewed By: dkgi

Differential Revision: D7985490

fbshipit-source-id: b6acdbbbbd1e2b14642eee388219a249c9f6f8ab
@dark
Copy link
Contributor

dark commented May 14, 2018

Version 0.0.6 will refuse to install on unsupported Python versions (<3.5 as of now).

@dkgi
Copy link
Contributor

dkgi commented May 14, 2018

This should be on pypi now. Closing

@dkgi dkgi closed this as completed May 14, 2018
facebook-github-bot pushed a commit that referenced this issue Aug 18, 2021
Summary:
More detailed discussion about allowing refinement to stick if there are no interleaving calls: https://fb.workplace.com/groups/657557118131740/permalink/907322746488508/

I'd like to get some input on a few different ways we could store "temporary" annotation information, ie., local type annotations that we know but want to be able to wipe away the next time we see any call.

There are a few layers where this might live.

1. Inside typecheck.ml as a new field in `State.t`
2. Inside resolution.ml as a new field in `t`.
3. Inside `annotation_store`, a field of `Resolution.t` - so instead of keeping one `RefinementUnit.t Reference.Map.t`, we'd keep around two of these.
4. Inside RefinementUnit.t as a flag on each base annotation
5. Inside Annotation.t as a new flag on immutable annotations.

I believe that #1 and #2 are too high level - we treat `annotation_store` as capturing all of the local type information at any state during type checking, and it seems messy to spill local typing information outside of this object. I could see this leading to bugs where we aren't joining everything necessary or remembering to check in two places for relevant local type info.

I believe #5 is considerable but potentially so low-level that it introduces too many headaches having to match out this extra complexity every time we deal with a basic annotation object.

I think #3 and #4 are the best potential options, and this diff sketches #3. Some tradeoffs:

#3 Pros/Cons:
(+) `annotation_store` still contains the full world local type information
(-) duplicate information needs to be stored. ie., if you have `a.b.c` as a temporary refinement, whereas `a.b` is a non-temporary local known type, you'll have an entry in both tables with the same refinement unit structure, just with the temporary table going a layer deeper. Any time the annotation store changes you'd want to wipe the corresponding entry in the temporary annotation store
(+) invalidating the whole thing is constant time - you can just set the temporary map to empty

#4 Pros/Cons:
(+) no data duplication
(+) simpler interface for `resolution.ml` - the most you'll need to expose is a flag on `set_local` on whether or not the set we're doing is a temporary refinement
(-) really expensive to invalidate, which honestly will be the overwhelming bulk of the operations done against this data store. We'll invalidate on every single call. As a result, I think the only way this is tenable is to add some kind of unfortunate extra complexity to make invalidation constant time, such as keeping a "validation counter" that ticks up by 1 when we want to invalidate, and keeping the "is_temporary" flag as an int instead of a bool. When we do the read, we can check whether the temporary data is still valid or not. I think this complexity makes this option a lot less appealing.

Thoughts on what the best option is? Is there a good model for this I didn't think of? Thanks in advance!

Reviewed By: grievejia

Differential Revision: D30351486

fbshipit-source-id: c25ae7bc663e625f5d7cfb84ad226d0456359cf8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform Everything related to the Operating System or the Python runtime Pyre is running on
Projects
None yet
Development

No branches or pull requests

4 participants