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

Python3.10 #21

Merged
merged 22 commits into from
Jul 15, 2022
Merged

Python3.10 #21

merged 22 commits into from
Jul 15, 2022

Conversation

chrisJohn404
Copy link
Contributor

@chrisJohn404 chrisJohn404 commented May 18, 2022

Python3.10

@chrisJohn404 chrisJohn404 changed the title Python3 Python3.10 May 18, 2022
@chrisJohn404
Copy link
Contributor Author

Sorry if this is to big of a pull request.

Copy link
Contributor

@sjarman28 sjarman28 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, it might be a while until we approve this request. Most of the changes look good, but ljsimpleregisterlookup is still on Python 2.7. If this request were approved, we would not be able to pull new changes from ljm_constants to ljsimpleregisterlookup without it breaking or doing some workaround, so we do not want to approve this request until ljsimpleregisterlookup is upgraded. A test deployment failed when I tried to upgrade ljsimpleregisterlookup to Python 3, and getting that working is a low priority at the moment.

@chrisJohn404
Copy link
Contributor Author

chrisJohn404 commented May 31, 2022

Not ideal, but reverted back to Python2; heroku docs?

Copy link
Contributor

@sjarman28 sjarman28 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything except the two os module imports I commented on look good to me.

About upgrading to Python 3:
I did upgrades in the backend for Python3, bumped the runtime to Python 3.10.4, and bumped some of the dependencies up to the newest release versions. The updated LJSimpleRegisterLookup works locally, but fails when trying to deploy. It appears to be a CORS header error, so there must be a configuration missing somewhere.

save_changes.py Outdated Show resolved Hide resolved
validate.py Show resolved Hide resolved
Copy link
Contributor

@sjarman28 sjarman28 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks.

@sjarman28 sjarman28 merged commit 28b9964 into labjack:master Jul 15, 2022
sjarman28 pushed a commit that referenced this pull request Jul 15, 2022
* Delete travis.yml

* Add MS github validation code.

* Addressed .json file issues.  Updated LabJackMModbusMap.h.  Updated the validator validator.  Updated validation code to support validation of the .json file.

* Added validation to save changes.

* Updates to *.h file for changes.

* Addressed comparison test TODO (ljmmm_test.py).

* Add build badge.

* Update README.md

* Indented validator code.

* Edited readme to reflect the use of github CI.

* Add validation checks for all included .json files. Improved error messages. Add .github validation

* Change name of validation step.

* Call path based operators through os to simplify imports.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants