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 failures in CI ahead of first release, drop Python 3.5 #11

Merged
merged 38 commits into from
Feb 14, 2020

Conversation

ulupo
Copy link
Collaborator

@ulupo ulupo commented Feb 10, 2020

What does this implement/fix? Explain your changes.

  • Drops support for Python 3.5 (changes in setup.py, azure-pipelines.yml and README.rst).
  • Edits administrative information contained in RELEASE.rst, GOVERNANCE.rst, setup.py, CODE_AUTHORS.rst, and LICENSE.
  • Adds a line to setup.cfg to avoid a pytest warning.
  • Tweaks doc/conf.py to avoid a warning.
  • Improves documentation and/or variable names in pyflagser/flagser.py and pyflagser/flagio.py.
  • Removes all nightly steps from azure-pipelines.yml as we are unlikely to do nightlies for this project.
  • Improves description and order of some steps in azure-pipelines.yml.
  • Creates a conftest.py file in pyflagser/tests where a command line option for pytest named --webdl is defined. When the user passes it, flag files are downloaded and stored in a temporary folder from a remote (public) bucket. This is useful if one wishes to run pytest on installed wheels, as the .flag files needed would not typically be found in ../../flagser/test relative to the location of the test files (e.g. pyflagser/tests/test_flagio.py). The use of pytest_generate_tests replaces the use of pytest.mark.parametrize in the test files as it is a well-known issue in pytest that fixtures cannot be passed to pytest.mark.parametrize, so the original idea of defining the locations of .flag files (whether they are downloaded or not) via a fixture then passed to pytest.mark.parametrize could not be implemented.
  • Creates a __main__.py and a conftest.py file in pyflagser/tests to be able to run tests on installed wheels using the command python -m pyflagser.tests --webdl --no-cov --no-coverage-upload. The reason pyflagser --pyargs pyflagser --webdl could not be made to work is that it is a well-known issue (see also here) that command-line options defined in a conftest cannot generically be found when pytest --pyargs is used.

@ulupo
Copy link
Collaborator Author

ulupo commented Feb 10, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ulupo
Copy link
Collaborator Author

ulupo commented Feb 10, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ulupo
Copy link
Collaborator Author

ulupo commented Feb 10, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ulupo
Copy link
Collaborator Author

ulupo commented Feb 10, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ulupo
Copy link
Collaborator Author

ulupo commented Feb 10, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ulupo
Copy link
Collaborator Author

ulupo commented Feb 11, 2020

I'm debugging this locally to see what I'm missing.

@gtauzin
Copy link
Collaborator

gtauzin commented Feb 11, 2020

Are we sure that when the wheels are being tested, the flagser directory is never available?

@ulupo
Copy link
Collaborator Author

ulupo commented Feb 11, 2020

Yes, this is what happens in local tests too. The point is that we are testing the shipped wheels and they do not come with the flag files from the flagser repo.

@ulupo
Copy link
Collaborator Author

ulupo commented Feb 11, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ulupo
Copy link
Collaborator Author

ulupo commented Feb 11, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ulupo
Copy link
Collaborator Author

ulupo commented Feb 12, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ulupo
Copy link
Collaborator Author

ulupo commented Feb 12, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ulupo
Copy link
Collaborator Author

ulupo commented Feb 12, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ulupo
Copy link
Collaborator Author

ulupo commented Feb 12, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ulupo
Copy link
Collaborator Author

ulupo commented Feb 13, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ulupo
Copy link
Collaborator Author

ulupo commented Feb 13, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ulupo ulupo changed the title [WIP] Fix failures in CI ahead of first release, drop Python 3.5 Fix failures in CI ahead of first release, drop Python 3.5 Feb 13, 2020
@ulupo
Copy link
Collaborator Author

ulupo commented Feb 13, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ulupo
Copy link
Collaborator Author

ulupo commented Feb 13, 2020

@gtauzin I believe this is ready for review.

Copy link
Collaborator Author

@ulupo ulupo left a comment

Choose a reason for hiding this comment

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

I have a remaining question @gtauzin. What is the meaning of the one-line description of saveflag?

Construct a sparse matrix from diagonals.

But it seems that a) this function's main purpose is to a actually save a .flag file; b) non-diagonal elements are also dealt with (?)

@ulupo ulupo requested a review from gtauzin February 13, 2020 08:52
@gtauzin gtauzin merged commit eb43890 into giotto-ai:master Feb 14, 2020
@ulupo ulupo deleted the CI/fix_failures branch February 14, 2020 12:54
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.

2 participants