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

Remove logging, handle test warnings, etc #922

Merged
merged 2 commits into from
Feb 17, 2022
Merged

Conversation

pllim
Copy link
Member

@pllim pllim commented Feb 10, 2022

This PR accomplishes the following:

xref spacetelescope/jdaviz#1084

@pllim pllim added this to the v1.x milestone Feb 10, 2022
@pllim pllim changed the title Remove logging Remove logging, handle test warnings Feb 10, 2022
@pllim pllim changed the title Remove logging, handle test warnings Remove logging, handle test warnings, etc Feb 10, 2022


@pytest.fixture
def simulated_spectra():
Copy link
Member Author

Choose a reason for hiding this comment

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

I moved the fixture here. This way, you don't have to import it and flake8 won't complain about the import.

pass


if sys.version_info < tuple((int(val) for val in __minimum_python_version__.split('.'))):
Copy link
Member Author

Choose a reason for hiding this comment

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

You don't need this. This is now defined in setup.cfg.

@@ -16,7 +15,7 @@
from ..registers import data_loader
from ..parsing_utils import _fits_identify_by_name, read_fileobj_or_hdulist

__all__ = ['spec_identify', 'spec_loader']
__all__ = ['identify_pfs_spec', 'pfs_spec_loader']
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure how this ever worked. This is a real bug.

@@ -87,19 +86,6 @@ def test_line_flux_masked():
assert quantity_allclose(result.value, 720.61116, atol=0.001)


def test_line_flux_uncertainty():
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a duplicate test.

* Move pytest fixture from tests/spectral_examples.py to conftest.py
* Remove unused imports and check for that in tox.ini
* Fix broken __all__ and check for that in tox.ini
* Move flake8 setting from tox.ini to setup.cfg for consistent behavior between IDE and CI. Change list from select to ignore. Fix some.
* Enable xfail_strict=true in testing.
* Other minor clean-ups
Copy link
Contributor

@rosteen rosteen left a comment

Choose a reason for hiding this comment

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

Thanks, I was getting tired of logging in different packages interfering with each other in weird ways, and I'm fine with a verbose flag for printing extra information. I appreciate the extra cleanup as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants