Skip to content

MRG, MAINT: Tweak CIs #7943

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

Merged
merged 13 commits into from
Jun 30, 2020
Merged

MRG, MAINT: Tweak CIs #7943

merged 13 commits into from
Jun 30, 2020

Conversation

larsoner
Copy link
Member

  1. Permit operators to start newlines in addition to at the end of lines (we have ~575 of these in master) so that we can transition to or at least allow the "new" (as of 2016) way of writing code, as discussed here
  2. Fix or ignore a few style errors in conf.py
  3. Run make flake on the entire codebase not just select folders
  4. Switch to conda for VTK (Following the installation instructions for macOS yields an error message #7939)
  5. Fix CircleCI failures related to numpydoc seen here

Closes #7939

NumpyDoc will still cause a failure, I think it needs to be fixed at the NumpyDoc end...

@larsoner
Copy link
Member Author

Fixed VTK9 rendering on macOS by disabling depth peeling and anti-aliasing. The remaining bug on my macOS system is that the widget controls do not work because there is a 2x mismatch between their render position and the click position needed to control them. We'll have to see if this is fixable at the PyVista end, or if this is an underlying VTK bug (my money is on the latter).

@larsoner larsoner changed the title WIP, MAINT: Tweak CIs MRG, MAINT: Tweak CIs Jun 29, 2020
@hoechenberger
Copy link
Member

The remaining bug on my macOS system is that the widget controls do not work because there is a 2x mismatch between their render position and the click position needed to control them. We'll have to see if this is fixable at the PyVista end, or if this is an underlying VTK bug (my money is on the latter).

This suggests that there's something off with the conversion between retina pixels and displayed points



###############################################################################
# Work around autosummary making bad :class: links
Copy link
Member

Choose a reason for hiding this comment

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

this sucks. It was working with an older version? if so can we rather pin the version rather than crippling our code?

Copy link
Member Author

Choose a reason for hiding this comment

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

How does this cripple our code? It just replaces a few docstrings. Subclassing like isinstance(x, dict) are still fine. Overhead should be minimal.

The problem is that numpydoc improved the signature extraction, and we hit a bug now with autosummary trying to add effectively:

:class:`v, remove specified key and return the corresponding value.`

to correspond to the D.pop() -> v, remove specified key and return the corresponding value. docstring.

This seemed like the cleanest solution, but I'm open to other ideas.

@agramfort
Copy link
Member

agramfort commented Jun 29, 2020 via email

@larsoner
Copy link
Member Author

See above -- I think it's because numpydoc passes the stdlib definition unchanged, then autodoc sees the -> in the docstring and assumes that anything after that is only a :class: (hence an autodoc/autosummary bug)

@agramfort
Copy link
Member

agramfort commented Jun 29, 2020 via email

@larsoner
Copy link
Member Author

Maybe, but in the meantime all CircleCI builds will fail. I guess we could roll back numpydoc in the meantime, but no idea how long the autodoc fix will take.

@larsoner
Copy link
Member Author

Personally I don't see why the dict_ class is so bad, it just adds one more entry in the MRO...

@agramfort
Copy link
Member

agramfort commented Jun 30, 2020 via email

@hoechenberger
Copy link
Member

I agree with @agramfort that it is kind of ugly, but I think it's okay for now :)

Copy link
Member

@hoechenberger hoechenberger left a comment

Choose a reason for hiding this comment

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

Wait, you're not doing that, unless my brain isn't functioning right bc I haven't had breakfast yet :) You're still using pip to install vtk, and only dropped the version constraint for macOS. This will not fix #7939.

@hoechenberger
Copy link
Member

hoechenberger commented Jun 30, 2020

I believe this also begs the question if / whether we should switch to conda-forge packages anyway (instead of using defaults / pkgs/main)

$ conda search vtk |grep pkgs/main
vtk                            8.1.0 py27h04d9db9_201  pkgs/main
vtk                            8.1.0 py27h87a2a2b_201  pkgs/main
vtk                            8.1.0 py35h04d9db9_201  pkgs/main
vtk                            8.1.0 py35h87a2a2b_201  pkgs/main
vtk                            8.1.0 py36h04d9db9_201  pkgs/main
vtk                            8.1.0 py36h87a2a2b_201  pkgs/main
vtk                            8.1.1 py27ha9eb873_204  pkgs/main
vtk                            8.1.1 py27hce9f6a2_204  pkgs/main
vtk                            8.1.1 py36ha9eb873_204  pkgs/main
vtk                            8.1.1 py36hce9f6a2_204  pkgs/main
vtk                            8.2.0 py27h9bafd54_200  pkgs/main
vtk                            8.2.0 py36h9bafd54_200  pkgs/main
vtk                            8.2.0 py37h9bafd54_200  pkgs/main
$

@hoechenberger
Copy link
Member

@larsoner feel free to revert 237e3e1, just want to see if this works…

@GuillaumeFavelier
Copy link
Contributor

@hoechenberger I think it follows the news on pyvista/pyvistaqt#16 (comment) and pyvista/pyvistaqt#15 (comment)

Fetching vtk 9.0.1 on PyPi for every OS could solve the problem.

@hoechenberger
Copy link
Member

Fetching vtk 9.0.1 on PyPi for every OS could solve the problem.

Ok so we only need to pin VTK to >=9.0.1?

@GuillaumeFavelier
Copy link
Contributor

For MacOS at least, yes

@GuillaumeFavelier
Copy link
Contributor

The failures of the notebook job are note related, see #7944

@GuillaumeFavelier
Copy link
Contributor

GuillaumeFavelier commented Jun 30, 2020

The Windows 3.7 conda job on Azure complains that pytest is not a command. It's not installed?

'pytest' is not recognized as an internal or external command,
operable program or batch file.

@agramfort
Copy link
Member

agramfort commented Jun 30, 2020 via email

@larsoner
Copy link
Member Author

The command failing is often an indicator that the install still failed (but didn't error out for some reason)

@GuillaumeFavelier
Copy link
Contributor

I pushed again, it seems there was indeed an error in the installation.

@GuillaumeFavelier
Copy link
Contributor

GuillaumeFavelier commented Jun 30, 2020

Still not happy:

# >>>>>>>>>>>>>>>>>>>>>> ERROR REPORT <<<<<<<<<<<<<<<<<<<<<<

    Traceback (most recent call last):
      File "d:\a\1\s\conda\lib\site-packages\conda\exceptions.py", line 1079, in __call__
        return func(*args, **kwargs)
      File "d:\a\1\s\conda\lib\site-packages\conda_env\cli\main.py", line 80, in do_call
        exit_code = getattr(module, func_name)(args, parser)
      File "d:\a\1\s\conda\lib\site-packages\conda_env\cli\main_update.py", line 68, in execute
        directory=os.getcwd())
      File "d:\a\1\s\conda\lib\site-packages\conda_env\specs\__init__.py", line 40, in detect
        if spec.can_handle():
      File "d:\a\1\s\conda\lib\site-packages\conda_env\specs\yaml_file.py", line 18, in can_handle
        self._environment = env.from_file(self.filename)
      File "d:\a\1\s\conda\lib\site-packages\conda_env\env.py", line 151, in from_file
        return from_yaml(yamlstr, filename=filename)
      File "d:\a\1\s\conda\lib\site-packages\conda_env\env.py", line 136, in from_yaml
        data = yaml_load_standard(yamlstr)
      File "d:\a\1\s\conda\lib\site-packages\conda\common\serialize.py", line 76, in yaml_load_standard
        return yaml.load(string, Loader=yaml.Loader, version="1.2")
      File "d:\a\1\s\conda\lib\site-packages\ruamel_yaml\main.py", line 640, in load
        return loader._constructor.get_single_data()  # type: ignore
      File "d:\a\1\s\conda\lib\site-packages\ruamel_yaml\constructor.py", line 102, in get_single_data
        node = self.composer.get_single_node()
      File "d:\a\1\s\conda\lib\site-packages\ruamel_yaml\composer.py", line 75, in get_single_node
        document = self.compose_document()
      File "d:\a\1\s\conda\lib\site-packages\ruamel_yaml\composer.py", line 96, in compose_document
        node = self.compose_node(None, None)
      File "d:\a\1\s\conda\lib\site-packages\ruamel_yaml\composer.py", line 132, in compose_node
        node = self.compose_mapping_node(anchor)
      File "d:\a\1\s\conda\lib\site-packages\ruamel_yaml\composer.py", line 194, in compose_mapping_node
        item_value = self.compose_node(node, item_key)
      File "d:\a\1\s\conda\lib\site-packages\ruamel_yaml\composer.py", line 130, in compose_node
        node = self.compose_sequence_node(anchor)
      File "d:\a\1\s\conda\lib\site-packages\ruamel_yaml\composer.py", line 163, in compose_sequence_node
        node.value.append(self.compose_node(node, index))
      File "d:\a\1\s\conda\lib\site-packages\ruamel_yaml\composer.py", line 132, in compose_node
        node = self.compose_mapping_node(anchor)
      File "d:\a\1\s\conda\lib\site-packages\ruamel_yaml\composer.py", line 194, in compose_mapping_node
        item_value = self.compose_node(node, item_key)
      File "d:\a\1\s\conda\lib\site-packages\ruamel_yaml\composer.py", line 130, in compose_node
        node = self.compose_sequence_node(anchor)
      File "d:\a\1\s\conda\lib\site-packages\ruamel_yaml\composer.py", line 162, in compose_sequence_node
        while not self.parser.check_event(SequenceEndEvent):
      File "d:\a\1\s\conda\lib\site-packages\ruamel_yaml\parser.py", line 144, in check_event
        self.current_event = self.state()
      File "d:\a\1\s\conda\lib\site-packages\ruamel_yaml\parser.py", line 519, in parse_indentless_sequence_entry
        token.move_comment(self.scanner.peek_token())
      File "d:\a\1\s\conda\lib\site-packages\ruamel_yaml\scanner.py", line 173, in peek_token
        self.fetch_more_tokens()
      File "d:\a\1\s\conda\lib\site-packages\ruamel_yaml\scanner.py", line 310, in fetch_more_tokens
        % utf8(ch), self.reader.get_mark())
    ruamel_yaml.scanner.ScannerError: while scanning for the next token
    found character '\t' that cannot start any token
      in "<unicode string>", line 35, column 44:
         ... 0.1; platform_system == "Darwin"		
                                             ^ (line: 35)

There is a tab in the file XD

@agramfort
Copy link
Member

how do we proceed? we have 2 PRs that are red but collectively could be green?

@GuillaumeFavelier
Copy link
Contributor

I branched out in #7944. If it makes things easier, I can integrate it in here?

@agramfort
Copy link
Member

agramfort commented Jun 30, 2020 via email

@agramfort agramfort merged commit b04ff04 into mne-tools:master Jun 30, 2020
@agramfort
Copy link
Member

thx a lot @larsoner and @GuillaumeFavelier !

@larsoner larsoner deleted the cis branch June 30, 2020 20:04
larsoner added a commit to larsoner/mne-python that referenced this pull request Jul 8, 2020
* upstream/master: (30 commits)
  MRG: Add remove_labels to _Brain (mne-tools#7964)
  Add get_picked_points (mne-tools#7963)
  ENH: Add OpenGL info to mne sys_info (mne-tools#7976)
  [MRG] Fix reject_tmin and reject_tmax for reject_by_annotation in mne.Epochs (mne-tools#7967)
  mrg: Add scalar mult and div operators for AverageTFR (mne-tools#7957)
  MRG, MAINT: Cleaner workaround for Sphinx linking issue (mne-tools#7970)
  MRG, ENH: Speed up epochs.copy (mne-tools#7968)
  MRG, BUG: Allow ref mags to have a comp grade (mne-tools#7965)
  do not forget to pass adjacency (mne-tools#7961)
  [MRG] fix Issue with stc.project after restricting to a label (mne-tools#7950)
  Only process nirx event file if present (mne-tools#7951)
  MRG+1: BUG: info['bads'] order shouldn't matter in write_evokeds() (mne-tools#7954)
  Fix some small glitches introduced via mne-tools#7845 (mne-tools#7952)
  Add time player (mne-tools#7940)
  MAINT: Clean up VTK9 offset array [circle front] (mne-tools#7953)
  MAINT: Skip a few more on macOS (mne-tools#7948)
  fix links [skip travis] (mne-tools#7949)
  MRG, MAINT: Tweak CIs (mne-tools#7943)
  MRG, BUG: Fix vector scaling (mne-tools#7934)
  MRG, VIZ, BUG: handle CSD channel type when topo plotting (mne-tools#7935)
  ...
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.

Following the installation instructions for macOS yields an error message
4 participants