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

Fix and test loading ORSO files #251

Merged
merged 5 commits into from
Jan 24, 2025
Merged

Fix and test loading ORSO files #251

merged 5 commits into from
Jan 24, 2025

Conversation

jmborr
Copy link
Contributor

@jmborr jmborr commented Jan 23, 2025

What was broken:

  • loading files using function load4. The import statement in webview's API was incorrect.

What is fixed:

  • Fix import statment of function load4 in webview's API
  • Unit test for funciton parse_orso()
  • add orsopy as a dependency

Check all that apply:

  • updated documentation
  • Source added/refactored
  • Added unit tests
  • Added integration tests

Check list for the reviewer

  • best software practices
    • clearly named variables (better to be verbose in variable names)
    • code comments explaining the intent of code blocks
  • All the tests are passing
  • The documentation is up to da

Signed-off-by: Jose Borreguero <borreguero@gmail.com>
Signed-off-by: Jose Borreguero <borreguero@gmail.com>
Signed-off-by: Jose Borreguero <borreguero@gmail.com>
@jmborr jmborr requested a review from glass-ships January 23, 2025 22:26
@jmborr jmborr self-assigned this Jan 23, 2025
@bmaranville
Copy link
Member

Can you describe what is broken, and then fixed by this PR?

@jmborr
Copy link
Contributor Author

jmborr commented Jan 24, 2025

Can you describe what is broken, and then fixed by this PR?

Will this work?

What was broken:

  • loading files using function load4. The import statement in webview's API was incorrect.

What is fixed:

  • Fix import statment of function load4 in webview's API
  • Unit test for funciton parse_orso()
  • add orsopy as a dependency

Signed-off-by: Jose Borreguero <borreguero@gmail.com>
Signed-off-by: Jose Borreguero <borreguero@gmail.com>
@glass-ships
Copy link
Member

@bmaranville do we have a preferred docstring style for this project?
I've noticed a couple different styles but it's hard to tell what the most common one used is,
especially given a lot of the latex expressions.

I lean toward google/numpy style, but if what jose has added in this PR is ok with you, I'm good with it.

@jmborr
Copy link
Contributor Author

jmborr commented Jan 24, 2025

I lean toward google/numpy style, but if what jose has added in this PR is ok with you, I'm good with it.

I always favor Numpy style docstrings. They occupy a bit more lines but perhaps that's the reason I find them easier to read 😃

@jmborr jmborr merged commit c05a0e4 into master Jan 24, 2025
12 checks passed
@jmborr
Copy link
Contributor Author

jmborr commented Jan 24, 2025

@bmaranville @glass-ships thanks for reviewing this

@glass-ships glass-ships deleted the import_load4 branch January 24, 2025 16:09
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.

3 participants