-
Notifications
You must be signed in to change notification settings - Fork 10
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
V4.y #38
V4.y #38
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this! I've never migrated a python project to use setup.cfg
. I have some changes and questions.
…using `pip install -e ".[option]"`. To add optional requirements add to the [optional.extras_require] in the setup config. All functions and imports work the same way. Restructured the testing suite so it follows a similar pattern. All tests pass.
…eprecation package.
…t. The versioneer part of the set up config will set the version and does not need to be written in the metadata section
…int then this is treated by urllib.parse like a full path.
…ignored by the current actions workflows.
…e image name to the correct one
Now finished work on the pyscicat version for interacting with the v4 scicat backend. Please see the following important changes to pyscicat: 1) Install and Setup 2) Deprecation of methods 3) Endpoint routes 4) Integration testing in github actions Any comments and review would be greatly appreciated. |
…clean break with pyscicat versions. If the method does not exist in the new backend it will be deleted from pyscicat.
@dylanmcreynolds why don't we align the version with the current SciCat one? |
One more thing: given that this PR goes against the v4.x branch, should we go ahead and merge and than work on the v4.x to make it production ready? |
Tests are failing because of the change in how requirements are listed. |
…from the setup config.
Hmm, flake8 failures now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pytest tests are failing in this PR because somewhere along the way in these many commits, the client.py
has changed dramatically.
test_client:test_scicat_ingest
fails because the returns forupload_instrument
,upload_proposal
andupload_sample
once returned a string, now they returns a dictionary (the ones you can see inadd_mock_reqeusts
. I don't know who uses these methods, but they return string in the main branch.- It looks like there are similar major changes that affect in
test_client2
SCICAT_PASSWORD - the password for your scicat user. | ||
""" | ||
|
||
sci_clie = ScicatClient(base_url=os.environ["BASE_URL"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This call to os.environ["BASE_URL"]
now blows up tests for developers that use pytest but don't have the full integration suite configured (like me). pytest picks up this module because it's name starts with test
.
I don't this module using either pytest or unittest. Perhaps we could change the module name from test_integration.py
to integration.py
to prevent is from getting in the way of pytest?
Please see #40 |
This is a first pass refactor to get pyscicat working with v4 of Scicat. We would like to :