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

Solve Windows bugs and CI #304

Merged
merged 2 commits into from
Dec 16, 2021
Merged

Solve Windows bugs and CI #304

merged 2 commits into from
Dec 16, 2021

Conversation

enekomartinmartinez
Copy link
Collaborator

@enekomartinmartinez enekomartinmartinez commented Dec 15, 2021

@JamesPHoughton this is ready for merge.

Some bugs were detected and solved for Windows:

  • encoding missing when loading submodules was giving an error.
  • the path of Macro objects was hardcoded from the root, this was making the models crash in Windows and would return an error if the model folder path is changed.

Improvements:

  • Enable using encoding information for reading Vensim model files.
  • Read modules files with "UTF-8" encoding.
  • Correct the paths and use pathlib.Path type object in the model file for external objects, modules and Macros.
  • Correct several tests to make them run on Windows also.
  • Skip some test on windows (invalid escape sequence error and CLI) to be corrected or replaced in the future.
  • Add windows to GitHub Actions CI.
  • Solve issue Circular reference error in models with ACTIVE INITIAL #305 of dependencies with active initial objects.

Closes #303 #305

@coveralls
Copy link

coveralls commented Dec 15, 2021

Coverage Status

Coverage increased (+0.001%) to 99.81% when pulling 6cc60ed on enekomartinmartinez:encoding-files into 54fa0f1 on JamesPHoughton:master.

@pep8speaks
Copy link

pep8speaks commented Dec 16, 2021

Hello @enekomartinmartinez! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 28:80: E501 line too long (105 > 79 characters)

Comment last updated at 2021-12-16 17:27:27 UTC

@enekomartinmartinez
Copy link
Collaborator Author

PR update with the solution for #305 adding running a new test model from test-models repo.
@JamesPHoughton

@enekomartinmartinez enekomartinmartinez linked an issue Dec 16, 2021 that may be closed by this pull request
Copy link
Collaborator

@JamesPHoughton JamesPHoughton 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 to me. =)

@enekomartinmartinez enekomartinmartinez merged commit fc06a12 into SDXorg:master Dec 16, 2021
@enekomartinmartinez enekomartinmartinez deleted the encoding-files branch January 3, 2022 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants