Skip to content

Conversation

@nh916
Copy link
Contributor

@nh916 nh916 commented Nov 1, 2023

Description

CRIPT Python SDK release v2.2.0 because there have been many minor change/improvements to the SDK both internal and external.

Changes

PR Template Changes

  • removed Tests section
  • added a section that states the developer tested their software and wrote appropriate tests for it

CI Upgrades

  • using Python 12 for CI jobs that require a single python version
  • CI runs Python 3.8 and 3.12

Documentation

  • added more sections to FAQ docs and fixed spelling errors
  • all code examples have been correct and I included CI into it
    • added doctest to all code examples
  • added wiki documentation link to README.md
  • fixed any missing/broken controlled vocabulary links
  • added BaseNode, UUIDBaseNode, and PrimaryBaseNode to documentation
    • in future releases, full documentation will be written for all the methods/functions/attributes of the base classes along with code examples with full doctest
  • wrote documentation for CRIPTDuplicateNameError exception & improved error message

Tests

  • renamed integrate_nodes_helper to save_integration_node_helper
  • reorganizing utility functions into test/utils/
  • made tests/ into a package to be easily imported in tests
  • created dynamic_material_data fixture for tests
  • testing notes attribute within all nodes
  • made error documentation headings more standard
    • from "How to Fix" to "Troubleshooting"

API Class

  • refactored get_s3_client into a function in a aws_s3_utils.py file
  • wrote docs for _prepare_host so it is clearer for future maintenance

Paginator

  • improved paginator error if API returns anything other than HTTP 200

Nodes

File Node

  • requiring file extension for the node

Material node

  • removed redundant Material name getter and setter

General

  • upgraded requirements.txt, requirements_docs.txt, requirements_dev.txt
  • CRIPT Python SDK requires Python 3.8+ as Python 3.7 has been depreciated
  • refactored APIError to be cleaner/more maintainable and gives better error
    • upgraded the documentation for it with Steps to try:
  • removed previously unfinished TODO comments

Feature

  • wrote API.delete_node_by_uuid(node_type, node_uuid) with docs and tests
  • wrote cript.API.get_node_by_uuid(uuid) with full docs and tests
    • reasoning for removing this feature before releasing it
      • essentially it was not robust and will lead to breaking changes in the near future
  • load_nodes_from_json works with both str and dict args, updated documentation for that
  • added feature to get expanded JSON without UUID
    • wrote feature, wrote comments, documentation, type hinting, doctests, and unit tests
  • wrote feature to skip validation
    • useful for when wanting to skip validations that slow the program down

Breaking Changes

The SDK will only work with Python 3.8 or higher as 3.7 has been depreciated

Acknowledgments 🎉

Big thanks to @InnocentBug for volunteering to help with the CRIPT Python SDK since its inception and continuing to add features, improve the SDK, and provide valuable support.

nh916 and others added 30 commits September 12, 2023 15:11
I've noticed test section has not been used in a while, and is just bloating the PR template lately, so  I'm thinking of removing it
* `How can I contribute to this project?` question was too long, so I chopped it into 2 sections instead
* added: `Is there documentation detailing the internal workings of the code?` question
* added the question: `Where can I find the release notes for each SDK version?`
* fixed grammar error in `Besides the user documentation, is there any developer documentation that I can read through on how
the code is written to get a better grasp of it?`
update PR template to remove the `Test` section
added link to wiki documentation to the documentation section of the README
upgraded GitHub action version `checkout` and `setup-python` to latest v4
added more sections to FAQ and fixed some grammar errors
* renamed `integrate_nodes_helper` to `save_integration_node_helper`
* changed all imports to be correct
* changed all function calls of `save_integration_node_helper` to be correct
* ran the tests from terminal and they all run fine
* put `integration_test_helper.py` into `tests/utils/`
* `util.py` into `tests/utils/`
  * the file might need renaming
* added `__init__.py` to make `tests/` into a package
* added `__init__.py` to make `utils/` into a package
* fixed imports to work correctly
* rang the tests from terminal and they all seem to be working fine
* upgraded trunk via terminal command `trunk upgrade`
## Changes
### renamed `integrate_nodes_helper` to `save_integration_node_helper`
* changed all imports to be correct
* changed all function calls of `save_integration_node_helper` to be correct
* ran the tests from terminal and they all run fine

### reorganizing utility functions into `test/utils/`
* put `integration_test_helper.py` into `tests/utils/`
* `util.py` into `tests/utils/`
  * the file might need renaming
* added `__init__.py` to make `tests/` into a package
* added `__init__.py` to make `utils/` into a package
* fixed imports to work correctly
* ran the tests from terminal and they all seem to be working fine
Python 3.7 is now deprecated

* changed README Python badge to be `3.8+`
* changed README installation guide to be `3.8+`
* changed installation documentation to require `Python 3.8+`
* changed `tests.yaml` to run from `[3.8, 3.11]`
* changed `setup.cfg` to require Python 3.8+
* clean up `APIError` so it is uniform everywhere and clean
* removed unnecessary code since it is being used uniformly everywhere
…xt` (#356)

fixed hot reload issue with MKDocs and upgraded outdated packages

* changed `watch` to be native in mkdocs.yml instead of `mkdocstrings`
  * `mkdocstrings` removed `watch` feature in `0.23.0` because MKDocs does it natively now and `mkdocstrings` has removed the feature
* upgraded documentation requirements_docs.txt
Feature: created `API.delete_node_by_uuid(node_type, node_uuid)`

* wrote documentation for `delete_node_by_uuid`
* changed `api.delete()` to call `api.delete_node_by_uuid` under the hood
* updated `api.delete()` documentation
* ran integration tests and it is working fine
Feature: created `cript.API.get_node_by_uuid(uuid)`

* wrote test for `get_node_by_uuid`
* wrote documentation for `get_node_by_uuid`
* made `test_api_search_get_node_by_uuid` DRY with uuid fixture
  * wrote `sodium_polystyrene_uuid` fixture
* made documentation example code easier to read by formatting the code
* putting `cript.API.search` parameters back to original because change was not needed
  * changed the parameter from `node_type` to `Union[str, node_type]` and later I saw that was unneeded and changed it back
* changing fixture decorator to be consistent with the rest
  * both work, but this is more consistent with the decorators we have written everywhere else
* formatted everything with black, trunk, and removed unused imports
* changed material fixture to be more abstract and more flexible
  * if we want to add more data to the fixture, we can just add it to the dictionary and all current tests will run fine.
* changed past tests to work with the new dynamic UUID fixture cleanly
upgraded APIError docs to have more steps in debugging/troubleshooting part
…ests

Upgrade API UUID tests to use dynamic UUID fixtures 

* changed material fixture to be more abstract and more flexible
  * if we want to add more data to the fixture, we can just add it to the dictionary and all current tests will run fine.
* changed past tests to work with the new dynamic UUID fixture cleanly
…362)

* refactored `get_s3_client` into a function in a aws_s3_utils.py file

* removed some bulk from the `cript.API` class
* wrote docstrings for `get_s3_client`
* tested the code and it is working fine
* formatted the whole thing with black
* changed log to have `file` be capitalized for better UI

* formatted with isort and then black

* fix trunk

---------

Co-authored-by: Ludwig Schneider <[email protected]>
renamed `_api_phandle` to `_api_prefix` because that is the correct term. This will make the code more self documenting and easier to read.
nh916 added 5 commits October 30, 2023 11:30
* removed and fixed some TODO comments

* upgraded `test_complex_process_node` to use fixture, removed unused fixtures, and removed copy.deep_copy()

* removed unused import `copy`
* `load_nodes_from_json` works with both str and dict args

this makes the function a more versatile and makes user scripts easier to write since they don't have to change formats.

The code written for it is pretty clean and nice to work with I think.

* added unit test for `load_nodes_from_json` with JSON dict argument and is passing

* updated docs code example for `load_nodes_from_json`
@trunk-io
Copy link

trunk-io bot commented Nov 1, 2023

Merging to develop in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

@nh916 nh916 marked this pull request as ready for review November 1, 2023 23:00
@nh916 nh916 marked this pull request as draft November 1, 2023 23:00
@nh916
Copy link
Contributor Author

nh916 commented Nov 2, 2023

@InnocentBug what are your thoughts on creating this release, and then in the next release removing the API.get_node_by_uuid() for in favor of API.get_node_by_exact_match()

This would be a bit of an inconvenience for the user because they could use API.get_node_by_uuid() and then it would be removed in the next release that would break their existing scripts, but it would get our release out faster and we can make the next release, I want to say pretty quickly, so it doesn't become too big of a problem. I can even put a small documentation on the API.get_node_by_uuid() telling the users that this will be replaced soon.

Not sure how long API.get_node_by_exact_match() would take to get ready currently

nh916 added 5 commits November 3, 2023 15:55
…d error message (#408)

* upgrade duplicate name error

The error was a bit harsh and unfinished, I was able to soften it and improve it.

* added documentation to `CRIPTDuplicateNameError`
* fixed docs heading `Create API Client with Environment Variables`
* fixed bad host example in docs
…emplate

added new checklist item to PR template about `automated tests`
@nh916 nh916 requested a review from InnocentBug November 13, 2023 17:41
* removed `get_node_by_uuid` cript.API feature

* removed unused import

* optimized imports with isort for api.py

* fix imports

---------

Co-authored-by: Ludwig Schneider <[email protected]>
@nh916 nh916 marked this pull request as ready for review November 14, 2023 23:49
Ludwig Schneider and others added 6 commits November 15, 2023 13:50
* add skip validation flag

* add tests

* force validation upon save

* respect global api in tests

* fix host copy

* more flexible return value of validation

* test for skipped validation None return
* test failure too

* accept kwargs

* wrote docs for `get_self_contained_json`

* formatted core.py with black

* added type hint of return type str

* renamed expanded json function

changed from `get_self_contained_json` to `get_expanded_json`

* refactored `get_expanded_json` test to work correctly with new name

* added comments
* renamed variables to be more self documenting and obvious
* added type hinting where it would help
* renamed test to be more self documenting and make more sense

* wrote docstrings for `test_expanded_json`

* added notes to `get_expanded_json`

* added link to GitHub discussions on deserialization

* added `linenums` to .cspell.json

* updated `get_expanded_json` code examples with better name

* changed name from `mybigsmiles` that triggered spelling error CI to something that would not trigger it and is more explicit and better anyways.
  * replaced it with `my material 1 bigsmiles` and `my material 2 bigsmiles`

* fixed extra parenthesis syntax error for doctest after merge

---------

Co-authored-by: nh916 <[email protected]>
fixed some small errors and mostly improved the code and docs
@InnocentBug
Copy link

Can we double check that the automated doctest runs as expected:

image

After that I am happy to approve.
I assume you ran the integration tests locally.

@nh916
Copy link
Contributor Author

nh916 commented Nov 16, 2023

Doctest

where are you seeing this error?

I think this may have been an issue that was happening before this PR: 8b8c02a#diff-ee69ed616853bef377a895f4f5c98f45386f3cf9553beaef9f1111670554a86cL354

I double checked it locally just now and I think it seems to be working fine, but double check and let me know

@InnocentBug
Copy link

double checked it locally just now and I think it seems to be working fine, but double check and let me know

this seems to be a temporary error in how github displays the logs

@nh916
Copy link
Contributor Author

nh916 commented Nov 17, 2023

double checked it locally just now and I think it seems to be working fine, but double check and let me know

this seems to be a temporary error in how github displays the logs

yeah that makes sense, I ran the tests locally and they all passed and the doctests are passing beautifully on the CI as well! Very well done job overall to both of us!

@nh916 nh916 merged commit f5126a6 into main Nov 17, 2023
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